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

fix: properly handle file/url paths on Windows #645

Merged
merged 1 commit into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions internal/output/sarif.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"text/template"

"github.com/google/osv-scanner/internal/url"
"github.com/google/osv-scanner/internal/utility/results"
"github.com/google/osv-scanner/internal/version"
"github.com/google/osv-scanner/pkg/models"
Expand Down Expand Up @@ -284,8 +285,11 @@ func PrintSARIFReport(vulnResult *models.VulnerabilityResults, outputWriter io.W
for pws := range gv.PkgSource {
artifactPath := stripGitHubWorkspace(pws.Source.Path)
if filepath.IsAbs(artifactPath) {
// Support absolute paths.
artifactPath = "file://" + artifactPath
// this only errors if the file path is not absolute,
// which we've already confirmed is not the case
p, _ := url.FromFilePath(artifactPath)

artifactPath = p.String()
}

run.AddDistinctArtifact(artifactPath)
Expand Down
7 changes: 6 additions & 1 deletion internal/sourceanalysis/go.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"

"github.com/google/osv-scanner/internal/sourceanalysis/govulncheck"
"github.com/google/osv-scanner/internal/url"
"github.com/google/osv-scanner/pkg/models"
"github.com/google/osv-scanner/pkg/reporter"
"golang.org/x/vuln/scan"
Expand Down Expand Up @@ -109,9 +110,13 @@ func runGovulncheck(moddir string, vulns []models.Vulnerability) (map[string][]*
}
}

// this only errors if the file path is not absolute,
// which paths from os.MkdirTemp should always be
dbdirURL, _ := url.FromFilePath(dbdir)

// Run govulncheck on the module at moddir and vulnerability database that
// was just created.
cmd := scan.Command(context.Background(), "-db", fmt.Sprintf("file://%s", dbdir), "-C", moddir, "-json", "./...")
cmd := scan.Command(context.Background(), "-db", dbdirURL.String(), "-C", moddir, "-json", "./...")
var b bytes.Buffer
cmd.Stdout = &b
if err := cmd.Start(); err != nil {
Expand Down
70 changes: 70 additions & 0 deletions internal/url/url.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package url

import (
"errors"
"net/url"
"path/filepath"
"strings"
)

// Code copied from https://github.com/golang/go/blob/7c2b69080a0b9e35174cc9c93497b6e7176f8275/src/cmd/go/internal/web/url.go
// TODO(golang.org/issue/32456): If accepted, move these functions into the
// net/url package.

var errNotAbsolute = errors.New("path is not absolute")

func FromFilePath(path string) (*url.URL, error) {
if !filepath.IsAbs(path) {
return nil, errNotAbsolute
}

// If path has a Windows volume name, convert the volume to a host and prefix
// per https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/.
if vol := filepath.VolumeName(path); vol != "" {
if strings.HasPrefix(vol, `\\`) {
path = filepath.ToSlash(path[2:])
i := strings.IndexByte(path, '/')

if i < 0 {
// A degenerate case.
// \\host.example.com (without a share name)
// becomes
// file://host.example.com/
return &url.URL{
Scheme: "file",
Host: path,
Path: "/",
}, nil
}

// \\host.example.com\Share\path\to\file
// becomes
// file://host.example.com/Share/path/to/file
return &url.URL{
Scheme: "file",
Host: path[:i],
Path: filepath.ToSlash(path[i:]),
}, nil
}

// C:\path\to\file
// becomes
// file:///C:/path/to/file
return &url.URL{
Scheme: "file",
Path: "/" + filepath.ToSlash(path),
}, nil
}

// /path/to/file
// becomes
// file:///path/to/file
return &url.URL{
Scheme: "file",
Path: filepath.ToSlash(path),
}, nil
}
38 changes: 38 additions & 0 deletions internal/url/url_other_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

//go:build !windows

package url

// Code copied from https://github.com/golang/go/blob/7c2b69080a0b9e35174cc9c93497b6e7176f8275/src/cmd/go/internal/web/url_other_test.go

var urlTests = []struct {
url string
filePath string
canonicalURL string // If empty, assume equal to url.
wantErr string
}{
// Examples from RFC 8089:
{
url: `file:///path/to/file`,
filePath: `/path/to/file`,
},
{
url: `file:/path/to/file`,
filePath: `/path/to/file`,
canonicalURL: `file:///path/to/file`,
},
{
url: `file://localhost/path/to/file`,
filePath: `/path/to/file`,
canonicalURL: `file:///path/to/file`,
},

// We reject non-local files.
{
url: `file://host.example.com/path/to/file`,
wantErr: "file URL specifies non-local host",
},
}
50 changes: 50 additions & 0 deletions internal/url/url_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package url

import (
"testing"
)

// Code copied from https://github.com/golang/go/blob/7c2b69080a0b9e35174cc9c93497b6e7176f8275/src/cmd/go/internal/web/url_test.go

func TestURLFromFilePath(t *testing.T) {
t.Parallel()

for _, tc := range urlTests {
if tc.filePath == "" {
continue
}
tc := tc

t.Run(tc.filePath, func(t *testing.T) {
t.Parallel()

u, err := FromFilePath(tc.filePath)
if err != nil {
if err.Error() == tc.wantErr {
return
}
if tc.wantErr == "" {
t.Fatalf("urlFromFilePath(%v): %v; want <nil>", tc.filePath, err)
} else {
t.Fatalf("urlFromFilePath(%v): %v; want %s", tc.filePath, err, tc.wantErr)
}
}

if tc.wantErr != "" {
t.Fatalf("urlFromFilePath(%v) = <nil>; want error: %s", tc.filePath, tc.wantErr)
}

wantURL := tc.url
if tc.canonicalURL != "" {
wantURL = tc.canonicalURL
}
if u.String() != wantURL {
t.Errorf("urlFromFilePath(%v) = %v; want %s", tc.filePath, u, wantURL)
}
})
}
}
96 changes: 96 additions & 0 deletions internal/url/url_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright 2019 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package url

// Code copied from https://github.com/golang/go/blob/7c2b69080a0b9e35174cc9c93497b6e7176f8275/src/cmd/go/internal/web/url_windows_test.go

var urlTests = []struct {
url string
filePath string
canonicalURL string // If empty, assume equal to url.
wantErr string
}{
// Examples from https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/:

{
url: `file://laptop/My%20Documents/FileSchemeURIs.doc`,
filePath: `\\laptop\My Documents\FileSchemeURIs.doc`,
},
{
url: `file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc`,
filePath: `C:\Documents and Settings\davris\FileSchemeURIs.doc`,
},
{
url: `file:///D:/Program%20Files/Viewer/startup.htm`,
filePath: `D:\Program Files\Viewer\startup.htm`,
},
{
url: `file:///C:/Program%20Files/Music/Web%20Sys/main.html?REQUEST=RADIO`,
filePath: `C:\Program Files\Music\Web Sys\main.html`,
canonicalURL: `file:///C:/Program%20Files/Music/Web%20Sys/main.html`,
},
{
url: `file://applib/products/a-b/abc_9/4148.920a/media/start.swf`,
filePath: `\\applib\products\a-b\abc_9\4148.920a\media\start.swf`,
},
{
url: `file:////applib/products/a%2Db/abc%5F9/4148.920a/media/start.swf`,
wantErr: "file URL missing drive letter",
},
{
url: `C:\Program Files\Music\Web Sys\main.html?REQUEST=RADIO`,
wantErr: "non-file URL",
},

// The example "file://D:\Program Files\Viewer\startup.htm" errors out in
// url.Parse, so we substitute a slash-based path for testing instead.
{
url: `file://D:/Program Files/Viewer/startup.htm`,
wantErr: "file URL encodes volume in host field: too few slashes?",
},

// The blog post discourages the use of non-ASCII characters because they
// depend on the user's current codepage. However, when we are working with Go
// strings we assume UTF-8 encoding, and our url package refuses to encode
// URLs to non-ASCII strings.
{
url: `file:///C:/exampleㄓ.txt`,
filePath: `C:\exampleㄓ.txt`,
canonicalURL: `file:///C:/example%E3%84%93.txt`,
},
{
url: `file:///C:/example%E3%84%93.txt`,
filePath: `C:\exampleㄓ.txt`,
},

// Examples from RFC 8089:

// We allow the drive-letter variation from section E.2, because it is
// simpler to support than not to. However, we do not generate the shorter
// form in the reverse direction.
{
url: `file:c:/path/to/file`,
filePath: `c:\path\to\file`,
canonicalURL: `file:///c:/path/to/file`,
},

// We encode the UNC share name as the authority following section E.3.1,
// because that is what the Microsoft blog post explicitly recommends.
{
url: `file://host.example.com/Share/path/to/file.txt`,
filePath: `\\host.example.com\Share\path\to\file.txt`,
},

// We decline the four- and five-slash variations from section E.3.2.
// The paths in these URLs would change meaning under path.Clean.
{
url: `file:////host.example.com/path/to/file`,
wantErr: "file URL missing drive letter",
},
{
url: `file://///host.example.com/path/to/file`,
wantErr: "file URL missing drive letter",
},
}