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

feat(c): add license support for conan lock files #6329

Merged
merged 14 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 9 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
19 changes: 13 additions & 6 deletions docs/docs/coverage/language/c.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,27 @@ Trivy supports [Conan][conan] C/C++ Package Manager.

The following scanners are supported.

| Package manager | SBOM | Vulnerability | License |
| --------------- | :---: | :-----------: | :-----: |
| Conan | ✓ | ✓ | - |
| Package manager | SBOM | Vulnerability | License |
|-----------------|:----:|:-------------:|:-------:|
| Conan | ✓ | ✓ | ✓[^1] |

The following table provides an outline of the features Trivy offers.

| Package manager | File | Transitive dependencies | Dev dependencies | [Dependency graph][dependency-graph] | Position |
| --------------- | -------------- | :---------------------: | :--------------: | :----------------------------------: | :------: |
| Conan | conan.lock[^1] | ✓ | Excluded | ✓ | ✓ |
|-----------------|----------------|:-----------------------:|:----------------:|:------------------------------------:|:--------:|
| Conan | conan.lock[^2] | ✓ | Excluded | ✓ | ✓ |

## Conan
In order to detect dependencies, Trivy searches for `conan.lock`[^1].

### Licenses
The Conan lock file doesn't contain any license information.
To obtain licenses we parse the `conanfile.py` files from the [conan cache directory][conan-cache-dir].
To correctly detection licenses, ensure that the cache directory contains all dependencies used.

[conan]: https://docs.conan.io/1/index.html
[conan-cache-dir]: https://docs.conan.io/1/mastering/custom_cache.html
[dependency-graph]: ../../configuration/reporting.md#show-origins-of-vulnerable-dependencies

[^1]: `conan.lock` is default name. To scan a custom filename use [file-patterns](../../configuration/skipping.md#file-patterns)
[^1]: The local cache should contain the dependencies used. See [licenses](#licenses).
[^2]: `conan.lock` is default name. To scan a custom filename use [file-patterns](../../configuration/skipping.md#file-patterns).
128 changes: 117 additions & 11 deletions pkg/fanal/analyzer/language/c/conan/conan.go
Original file line number Diff line number Diff line change
@@ -1,42 +1,148 @@
package conan

import (
"bufio"
"context"
"io"
"io/fs"
"os"
"path"
"path/filepath"
"sort"
"strings"

"golang.org/x/xerrors"

"github.com/aquasecurity/trivy/pkg/dependency/parser/c/conan"
godeptypes "github.com/aquasecurity/trivy/pkg/dependency/types"
"github.com/aquasecurity/trivy/pkg/fanal/analyzer"
"github.com/aquasecurity/trivy/pkg/fanal/analyzer/language"
"github.com/aquasecurity/trivy/pkg/fanal/types"
"github.com/aquasecurity/trivy/pkg/log"
"github.com/aquasecurity/trivy/pkg/utils/fsutils"
)

func init() {
analyzer.RegisterAnalyzer(&conanLockAnalyzer{})
analyzer.RegisterPostAnalyzer(analyzer.TypeConanLock, newConanLockAnalyzer)
}

const (
version = 1
version = 2
)

// conanLockAnalyzer analyzes conan.lock
type conanLockAnalyzer struct{}
type conanLockAnalyzer struct {
logger *log.Logger
parser godeptypes.Parser
}

func newConanLockAnalyzer(_ analyzer.AnalyzerOptions) (analyzer.PostAnalyzer, error) {
return conanLockAnalyzer{
logger: log.WithPrefix("conan"),
parser: conan.NewParser(),
}, nil
}

func (a conanLockAnalyzer) Analyze(_ context.Context, input analyzer.AnalysisInput) (*analyzer.AnalysisResult, error) {
p := conan.NewParser()
res, err := language.Analyze(types.Conan, input.FilePath, input.Content, p)
func (a conanLockAnalyzer) PostAnalyze(_ context.Context, input analyzer.PostAnalysisInput) (*analyzer.AnalysisResult, error) {
required := func(filePath string, d fs.DirEntry) bool {
return a.Required(filePath, nil)
}

licenses, err := licensesFromCache()
if err != nil {
return nil, xerrors.Errorf("%s parse error: %w", input.FilePath, err)
a.logger.Debug("Unable to parse cache directory to obtain licenses", log.Err(err))
}

var apps []types.Application
if err = fsutils.WalkDir(input.FS, ".", required, func(filePath string, _ fs.DirEntry, r io.Reader) error {
app, err := language.Parse(types.Conan, filePath, r, a.parser)
if err != nil {
return xerrors.Errorf("%s parse error: %w", filePath, err)
}

if app == nil {
return nil
}

// Fill licenses
for i, lib := range app.Libraries {
if license, ok := licenses[lib.Name]; ok {
app.Libraries[i].Licenses = []string{
license,
}
}
}

sort.Sort(app.Libraries)
apps = append(apps, *app)
return nil
}); err != nil {
return nil, xerrors.Errorf("unable to parse conan lock file: %w", err)
}

return &analyzer.AnalysisResult{
Applications: apps,
}, nil
}

func licensesFromCache() (map[string]string, error) {
required := func(filePath string, d fs.DirEntry) bool {
return filepath.Base(filePath) == "conanfile.py"
Copy link
Collaborator

@knqyf263 knqyf263 Apr 23, 2024

Choose a reason for hiding this comment

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

Isn't conanfile.txt used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cache dir contains only conanfile.py files:

root@3fcddc7fe2e5:/app# find /root/.conan -name "conanfile.*"
/root/.conan/data/zlib/1.3.1/_/_/export/conanfile.py

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is conanfile.py required? I'm wondering if we just don't find a project using conanfile.txt as most projects use conanfile.py. But we can handle conanfile.txt if we find such a case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIUC only conanfile.py contains attributes - https://docs.conan.io/2/reference/conanfile_txt.html#conanfile-txt

Therefore, we can't detect package name/license from conanfile.txt files.
Then we don't need to parse the conanfile.txt files.

}

// cf. https://docs.conan.io/1/mastering/custom_cache.html
cacheDir := os.Getenv("CONAN_USER_HOME")
if cacheDir == "" {
cacheDir, _ = os.UserHomeDir()
}
cacheDir = path.Join(cacheDir, ".conan", "data")

if !fsutils.DirExists(cacheDir) {
return nil, xerrors.Errorf("the Conan cache directory (%s) was not found.", cacheDir)
}

licenses := make(map[string]string)
if err := fsutils.WalkDir(os.DirFS(cacheDir), ".", required, func(filePath string, _ fs.DirEntry, r io.Reader) error {
scanner := bufio.NewScanner(r)
var name, license string
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())

if strings.HasPrefix(line, "name") { // cf. https://docs.conan.io/1/reference/conanfile/attributes.html#name
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

// trim extra characters - e.g. `name = "openssl"` -> `openssl`
name = strings.TrimSuffix(strings.TrimPrefix(line, `name = "`), `"`)
// Check that the license is already found
if license != "" {
break
}
} else if strings.HasPrefix(line, "license") { // cf. https://docs.conan.io/1/reference/conanfile/attributes.html#license
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be more strict. Otherwise, it matches other than the license line. That's why I suggested regexp this time.
https://github.com/conan-io/conan-center-index/blob/55703762e2ca0560fd44d9008e00e61a0a64b23c/recipes/zlib/all/conanfile.py#L90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it looks like we can't avoid using regexp...

Copy link
Collaborator

Choose a reason for hiding this comment

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

If possible, I don't want to use regexp. I'm still considering a way to parse the license line without regexp, but...

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Apr 23, 2024

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, it's better.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Apr 23, 2024

Choose a reason for hiding this comment

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

used strings.Cut - 9b5d8ce + 7bedafb

// trim extra characters - e.g. `license = "Apache-2.0"` -> `Apache-2.0`
license = strings.TrimSuffix(strings.TrimPrefix(line, `license = "`), `"`)
Copy link
Collaborator

@knqyf263 knqyf263 Apr 22, 2024

Choose a reason for hiding this comment

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

I found this example in the doc.

from conan import ConanFile

class MyAndroidNDKRecipe(ConanFile):

  name="my_android_ndk"
  version="1.0"

  def package_info(self):
      self.conf_info.define("tools.android:ndk_path", "bar")

We probably need a regexp like name\s*=\s*"([^"]+)". I want to avoid using regular expressions wherever possible, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In addition, if a conanfile.py is small enough, applying regexp to the file content might be simpler and faster.

    var (
        namePattern = regexp.MustCompile(`^\s*name\s*=\s*"(?P<name>[^"]+)"$`)
        licensePattern = regexp.MustCompile(`^\s*license\s*=\s*"(?P<license>[^"]+)"$`)
    )
    ...

    content, err := io.ReadAll(r)
    if err != nil {...}

    // Find name
    if match := namePattern.FindSubmatch(content); len(match) > 0 {
        name = string(match[namePattern.SubexpIndex("name")])
    }

    // Find license
    if match := licensePattern.FindSubmatch(content); if len(match) > 0 {
        license = string(match[licensePattern.SubexpIndex("version")])
    }

I am not 100% sure which is faster as I have not compared it with processing one line at a time. I was just thinking out loud, and it's not a performance critical process. You can decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

name="my_android_ndk"

It looks as bug, but we can also parse this case.

We probably need a regexp like name\s*=\s*"([^"]+)". I want to avoid using regular expressions wherever possible, though.

I also don't trust regexp and try not to use them.
I think we can just remove all the spaces.

I am not 100% sure which is faster as I have not compared it with processing one line at a time.

In the files that I checked, name and license fields are placed in first 20-30 lines.
So I think we won't gain much time by doing this.

if a conanfile.py is small enough, applying regexp to the file content might be simpler and faster.

675 lines - https://github.com/aquasecurity/trivy/blob/61af875ec3120ca1fcbae2a7e684173b40ca71b5/pkg/fanal/analyzer/language/c/conan/testdata/cacheDir/.conan/data/openssl/3.0.5/_/_/export/conanfile.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added removing extra spaces - 89003d6

Copy link
Collaborator

@knqyf263 knqyf263 Apr 23, 2024

Choose a reason for hiding this comment

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

It looks as bug, but we can also parse this case.

Why is it a bug? It does not break the Python semantics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen such cases. Also, all test cases use the format name = "foo". Therefore, I assumed that this was a bug or maybe just a typo in the documentation.

In any case, this is easy to solve.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Conanfile is written by hand, so any format is possible, isn't it? I think there is a customary preferred format, but it is legitimate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is a customary preferred format, but it is legitimate.

You are right 👍

// Check that the name is already found
if name != "" {
break
}
}
}

// Skip files without name/license
if name == "" || license == "" {
return nil
}
knqyf263 marked this conversation as resolved.
Show resolved Hide resolved

licenses[name] = license
return nil
}); err != nil {
return nil, xerrors.Errorf("the Conan cache dir (%s) walk error: %w", cacheDir, err)
}
return res, nil
return licenses, nil
}

func (a conanLockAnalyzer) Required(_ string, fileInfo os.FileInfo) bool {
func (a conanLockAnalyzer) Required(filePath string, _ os.FileInfo) bool {
// Lock file name can be anything
// cf. https://docs.conan.io/en/latest/versioning/lockfiles/introduction.html#locking-dependencies
// cf. https://docs.conan.io/1/versioning/lockfiles/introduction.html#locking-dependencies
// By default, we only check the default filename - `conan.lock`.
return fileInfo.Name() == types.ConanLock
return filepath.Base(filePath) == types.ConanLock
}

func (a conanLockAnalyzer) Type() analyzer.Type {
Expand Down
100 changes: 67 additions & 33 deletions pkg/fanal/analyzer/language/c/conan/conan_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package conan

import (
"context"
"os"
"path/filepath"
"sort"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -15,18 +14,19 @@ import (

func Test_conanLockAnalyzer_Analyze(t *testing.T) {
tests := []struct {
name string
inputFile string
want *analyzer.AnalysisResult
name string
dir string
cacheDir string
want *analyzer.AnalysisResult
}{
{
name: "happy path",
inputFile: "testdata/happy.lock",
name: "happy path",
dir: "testdata/happy",
want: &analyzer.AnalysisResult{
Applications: []types.Application{
{
Type: types.Conan,
FilePath: "testdata/happy.lock",
FilePath: "conan.lock",
Libraries: types.Packages{
{
ID: "openssl/3.0.5",
Expand Down Expand Up @@ -60,31 +60,73 @@ func Test_conanLockAnalyzer_Analyze(t *testing.T) {
},
},
{
name: "empty file",
inputFile: "testdata/empty.lock",
name: "happy path with cache dir",
dir: "testdata/happy",
cacheDir: "testdata/cacheDir",
want: &analyzer.AnalysisResult{
Applications: []types.Application{
{
Type: types.Conan,
FilePath: "conan.lock",
Libraries: types.Packages{
{
ID: "openssl/3.0.5",
Name: "openssl",
Version: "3.0.5",
Licenses: []string{
"Apache-2.0",
},
DependsOn: []string{
"zlib/1.2.12",
},
Locations: []types.Location{
{
StartLine: 12,
EndLine: 21,
},
},
},
{
ID: "zlib/1.2.12",
Name: "zlib",
Version: "1.2.12",
Licenses: []string{
"Zlib",
},
Indirect: true,
Locations: []types.Location{
{
StartLine: 22,
EndLine: 28,
},
},
},
},
},
},
},
},
{
name: "empty file",
dir: "testdata/empty",
want: &analyzer.AnalysisResult{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f, err := os.Open(tt.inputFile)
if tt.cacheDir != "" {
t.Setenv("CONAN_USER_HOME", tt.cacheDir)
}
a, err := newConanLockAnalyzer(analyzer.AnalyzerOptions{})
require.NoError(t, err)
defer f.Close()

a := conanLockAnalyzer{}
got, err := a.Analyze(nil, analyzer.AnalysisInput{
FilePath: tt.inputFile,
Content: f,
got, err := a.PostAnalyze(context.Background(), analyzer.PostAnalysisInput{
FS: os.DirFS(tt.dir),
})

if got != nil {
for _, app := range got.Applications {
sort.Sort(app.Libraries)
}
}

assert.NoError(t, err)
assert.Equal(t, tt.want, got)
require.NoError(t, err)
require.Equal(t, tt.want, got)
})
}
}
Expand Down Expand Up @@ -113,16 +155,8 @@ func Test_conanLockAnalyzer_Required(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
dir := t.TempDir()
f, err := os.Create(filepath.Join(dir, tt.filePath))
require.NoError(t, err)
defer f.Close()

fi, err := f.Stat()
require.NoError(t, err)

a := conanLockAnalyzer{}
got := a.Required("", fi)
got := a.Required(tt.filePath, nil)
assert.Equal(t, tt.want, got)
})
}
Expand Down
Loading