Add TravisCI source #1877

dustin-decker · 2023-10-09T02:44:44Z

Adds Travis CI scanning

Checklist:

Tests passing (make test-community)?
Lint passing (make lint this requires golangci-lint)?

…vis-scanning

ListByBuild does not support pagination, so this was infinitely repeating. https://developer.travis-ci.com/resource/jobs#find

rosecodym

it looks like there's some circleci stuff that hasn't been updated after a copy-paste (annotated inline). i'll also leave the units stuff to @mcastorina to review.

rosecodym · 2023-10-17T14:45:43Z

pkg/engine/engine.go

@@ -306,6 +306,7 @@ func Start(ctx context.Context, options ...EngineOption) (*Engine, error) {
 	e.sourceManager = sources.NewManager(
 		sources.WithConcurrentSources(int(e.concurrency)),
 		sources.WithConcurrentUnits(int(e.concurrency)),
+		sources.WithSourceUnits(),


@mcastorina are we ready for this?

Yeah, this tells the manager to use SourceUnits only if the source supports it, which right now would be filesystem and travisci.

pkg/engine/travisci.go

pkg/sources/travisci/travisci.go

rosecodym · 2023-10-17T14:54:19Z

pkg/sources/travisci/travisci.go

+	if err != nil {
+		return fmt.Errorf("error finding repository: %w", err)
+	}
+	logger := ctx.Logger().WithValues("repo", *repo.Name)


are we 100% confident that repo is non-nil here? (i think this is just a go idiom i'm still becoming accustomed to.)

also, does the context logger already have kvps that carry information about the source itself? those might be nice to add if not.

No, it depends on the implementation but that's generally the idiom.

Re: logger, I'm not sure but should to caller populate those (source id etc) consistently rather than the source being responsible?

Tested the logger locally and this is what's printed, so it already has context (though missing source_id it seems).

{ "level": "info-2", "ts": "2023-10-18T11:54:13-07:00", "logger": "trufflehog", "msg": "scanning repository", "job_id": 1, "source_manager_worker_id": "G26hU", "source_type": "SOURCE_TYPE_TRAVISCI", "source_name": "trufflehog - Travis CI", "unit": "26155351", "repo": "test-repo" }

No, it depends on the implementation but that's generally the idiom.

If we're not 100% confident that repo won't be nil (maybe Find is misbehaving, I guess) do you think it's worth adding a nil check for repo to avoid a potential panic?

rosecodym · 2023-10-17T14:58:17Z

pkg/sources/travisci/travisci.go

+								BuildNumber: *build.Number,
+								JobNumber:   *job.Number,
+								Link:        fmt.Sprintf("https://app.travis-ci.com/github/%s/%s/jobs/%d", *job.Owner.Login, *repo.Name, *job.Id),
+								Public:      !*repo.Private,


out of curiosity, why did we invert this property?

What is inverted?

source_metadatapb.TravisCI has a Public property but repo has a Private property

I did this to stay more consistent with other source metadata.

rosecodym · 2023-10-17T15:00:54Z

pkg/sources/travisci/travisci_test.go

+			}
+
+			for _, chunk := range reporter.Chunks {
+				if diff := pretty.Compare(chunk.SourceMetadata, tt.wantSourceMetadata); diff != "" {


we have been switching the notifiers over to use cmp because of that package's ability to exclude fields from the comparison. i can see that you don't need that capability here but the consistency could be nice. (cmp is also a google package, for what that's worth.)

pkg/sources/travisci/travisci.go

ahrav · 2023-10-18T13:43:15Z

pkg/sources/travisci/travisci.go

+				return err
+			}
+			buildPageErrs++
+			if buildPageErrs >= 5 {


nit: Could we constant-ify the 5.

rosecodym

circleci stuff

rosecodym · 2023-10-19T14:40:10Z

pkg/sources/travisci/travisci.go

+	if err != nil {
+		return fmt.Errorf("error finding repository: %w", err)
+	}
+	logger := ctx.Logger().WithValues("repo", *repo.Name)


No, it depends on the implementation but that's generally the idiom.

If we're not 100% confident that repo won't be nil (maybe Find is misbehaving, I guess) do you think it's worth adding a nil check for repo to avoid a potential panic?

rosecodym · 2023-10-19T14:40:59Z

pkg/sources/travisci/travisci.go

+								BuildNumber: *build.Number,
+								JobNumber:   *job.Number,
+								Link:        fmt.Sprintf("https://app.travis-ci.com/github/%s/%s/jobs/%d", *job.Owner.Login, *repo.Name, *job.Id),
+								Public:      !*repo.Private,


source_metadatapb.TravisCI has a Public property but repo has a Private property

…vis-scanning

Add TravisCI source

b960231

dustin-decker mentioned this pull request Oct 10, 2023

[chore] Add SourceUnitEnumChunker filesystem tests #1873

Merged

2 tasks

dustin-decker and others added 4 commits October 16, 2023 13:42

Merge branch 'main' of github.com:trufflesecurity/trufflehog into tra…

c99e0b9

…vis-scanning

update test to use sourcestest

ec3c59d

Remove jobPage loop

e5a34a9

ListByBuild does not support pagination, so this was infinitely repeating. https://developer.travis-ci.com/resource/jobs#find

Continue chunking on error

fe90eff

dustin-decker marked this pull request as ready for review October 16, 2023 23:51

dustin-decker requested review from a team as code owners October 16, 2023 23:51

rosecodym reviewed Oct 17, 2023

View reviewed changes

ahrav approved these changes Oct 18, 2023

View reviewed changes

rosecodym requested changes Oct 18, 2023

View reviewed changes

review updates

065984e

rosecodym approved these changes Oct 19, 2023

View reviewed changes

dustin-decker added 2 commits October 30, 2023 07:23

update readme

628a55d

Merge branch 'main' of github.com:trufflesecurity/trufflehog into tra…

d707ef5

…vis-scanning

dustin-decker merged commit 05fae15 into main Oct 30, 2023
7 of 8 checks passed

dustin-decker deleted the travis-scanning branch October 30, 2023 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TravisCI source #1877

Add TravisCI source #1877

dustin-decker commented Oct 9, 2023 •

edited

Loading

rosecodym left a comment

rosecodym Oct 17, 2023

mcastorina Oct 17, 2023

rosecodym Oct 17, 2023

rosecodym Oct 17, 2023

dustin-decker Oct 18, 2023

mcastorina Oct 18, 2023

rosecodym Oct 19, 2023

rosecodym Oct 17, 2023

dustin-decker Oct 18, 2023

rosecodym Oct 19, 2023

dustin-decker Oct 30, 2023

rosecodym Oct 17, 2023

ahrav Oct 18, 2023

rosecodym left a comment

rosecodym Oct 19, 2023

rosecodym Oct 19, 2023

Add TravisCI source #1877

Add TravisCI source #1877

Conversation

dustin-decker commented Oct 9, 2023 • edited Loading

Checklist:

rosecodym left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosecodym left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustin-decker commented Oct 9, 2023 •

edited

Loading