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.

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

fix(nodejs): add name validation for package name from package.json #6268

Merged
merged 3 commits into from
Mar 11, 2024
Merged

fix(nodejs): add name validation for package name from package.json #6268

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
c0835e3
fix(packagejson): add name validation
DmitriyLewen Mar 5, 2024
9db4037
fix mistake
DmitriyLewen Mar 5, 2024
0b78e31
refactor nameRegexp
DmitriyLewen Mar 5, 2024
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
16 changes: 16 additions & 0 deletions pkg/dependency/parser/nodejs/packagejson/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ package packagejson
import (
"encoding/json"
"io"
"regexp"

"golang.org/x/xerrors"

"github.com/aquasecurity/trivy/pkg/dependency/parser/types"
"github.com/aquasecurity/trivy/pkg/dependency/parser/utils"
)

var nameRegexp = regexp.MustCompile(`^(@[A-Za-z0-9-._]+/)?[A-Za-z0-9-._]+$`)

type packageJSON struct {
Name string `json:"name"`
Version string `json:"version"`
Expand Down Expand Up @@ -40,6 +43,10 @@ func (p *Parser) Parse(r io.Reader) (Package, error) {
return Package{}, xerrors.Errorf("JSON decode error: %w", err)
}

if !IsValidName(pkgJSON.Name) {
return Package{}, xerrors.Errorf("Name can only contain URL-friendly characters")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the warning message is enough. Otherwise, if a package.json file contains an invalid name for testing, it fails the whole scanning.

Copy link
Collaborator

@knqyf263 knqyf263 Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Or is it anyway converted into a warning in analyzer.go? If so, never mind my comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or is it anyway converted into a warning in analyzer.go

Right.

}

var id string
// Name and version fields are optional
// https://docs.npmjs.com/cli/v9/configuring-npm/package-json#name
Expand Down Expand Up @@ -73,3 +80,12 @@ func parseLicense(val interface{}) string {
}
return ""
}

func IsValidName(name string) bool {
// Name is optional field
// https://docs.npmjs.com/cli/v9/configuring-npm/package-json#name
if name == "" {
return true
}
return nameRegexp.MatchString(name)
}
58 changes: 56 additions & 2 deletions pkg/dependency/parser/nodejs/packagejson/parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package packagejson_test

import (
"os"
"path"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -77,6 +76,11 @@ func TestParse(t *testing.T) {
},
},
},
{
name: "invalid package name",
inputFile: "testdata/invalid_name.json",
wantErr: "Name can only contain URL-friendly characters",
},
{
name: "sad path",
inputFile: "testdata/invalid_package.json",
Expand All @@ -99,7 +103,7 @@ func TestParse(t *testing.T) {
}

for _, v := range vectors {
t.Run(path.Base(v.name), func(t *testing.T) {
t.Run(v.name, func(t *testing.T) {
f, err := os.Open(v.inputFile)
require.NoError(t, err)
defer f.Close()
Expand All @@ -115,3 +119,53 @@ func TestParse(t *testing.T) {
})
}
}

func TestIsValidName(t *testing.T) {
tests := []struct {
name string
want bool
}{
{
name: "",
want: true,
},
{
name: "test_package",
want: true,
},
{
name: "test.package",
want: true,
},
{
name: "test-package",
want: true,
},
{
name: "@test/package",
want: true,
},
{
name: "test@package",
want: false,
}, {
name: "test?package",
want: false,
},
{
name: "test/package",
want: false,
},
{
name: "package/",
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
valid := packagejson.IsValidName(tt.name)
require.Equal(t, tt.want, valid)
})
}
}
11 changes: 11 additions & 0 deletions pkg/dependency/parser/nodejs/packagejson/testdata/invalid_name.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@invalid/packageName/",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"author": "",
"license": "ISC"
}