From 25fa0f3fb29de0670f402b5b7ea464c8cd68005d Mon Sep 17 00:00:00 2001 From: Fabian Kammel Date: Wed, 21 Dec 2022 00:52:08 +0100 Subject: [PATCH] ci: Enable and fix lll (#1395) Signed-off-by: Fabian Kammel --- .golangci.yml | 3 +- github/oidc.go | 6 +++- internal/builders/container/generate.go | 6 +++- internal/builders/docker/commands.go | 3 +- internal/builders/docker/pkg/common.go | 1 + internal/builders/generic/attest.go | 14 ++++++-- internal/builders/generic/attest_test.go | 38 ++++++++++++++------- internal/builders/go/pkg/build_test.go | 6 ++-- internal/builders/go/pkg/provenance.go | 9 +++-- internal/builders/go/pkg/provenance_test.go | 6 +++- internal/builders/nodejs/attest.go | 4 ++- internal/utils/marshal_test.go | 6 ++-- 12 files changed, 73 insertions(+), 29 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8f78552344..480a84f591 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -59,8 +59,7 @@ linters: # TODO(github.com/slsa-framework/slsa-github-generator/issues/450): enable govet # - govet - ineffassign - # TODO(github.com/slsa-framework/slsa-github-generator/issues/450): enable lll - # - lll + - lll - makezero - misspell - nakedret diff --git a/github/oidc.go b/github/oidc.go index 012b291bbe..503b5eed14 100644 --- a/github/oidc.go +++ b/github/oidc.go @@ -104,7 +104,11 @@ func NewOIDCClient() (*OIDCClient, error) { requestURL := os.Getenv(requestURLEnvKey) parsedURL, err := url.ParseRequestURI(requestURL) if err != nil { - return nil, errors.Errorf(&errURLError{}, "invalid request URL %q: %w; does your workflow have `id-token: write` scope?", requestURL, err) + return nil, errors.Errorf( + &errURLError{}, + "invalid request URL %q: %w; does your workflow have `id-token: write` scope?", + requestURL, err, + ) } c := OIDCClient{ diff --git a/internal/builders/container/generate.go b/internal/builders/container/generate.go index d93c95f109..12d7a9767c 100644 --- a/internal/builders/container/generate.go +++ b/internal/builders/container/generate.go @@ -82,7 +82,11 @@ that it is being run in the context of a Github Actions workflow.`, }, } - c.Flags().StringVarP(&predicatePath, "predicate", "p", "predicate.json", "Path to write the unsigned provenance predicate.") + c.Flags().StringVarP( + &predicatePath, + "predicate", "p", "predicate.json", + "Path to write the unsigned provenance predicate.", + ) return c } diff --git a/internal/builders/docker/commands.go b/internal/builders/docker/commands.go index 3774a2c8f2..532f3420db 100644 --- a/internal/builders/docker/commands.go +++ b/internal/builders/docker/commands.go @@ -37,7 +37,8 @@ type InputOptions struct { // AddFlags adds input flags to the given command. func (o *InputOptions) AddFlags(cmd *cobra.Command) { - cmd.Flags().StringVarP(&o.BuildConfigPath, "build-config-path", "c", "", "Required - Path to a toml file containing the build configs.") + cmd.Flags().StringVarP(&o.BuildConfigPath, "build-config-path", "c", "", + "Required - Path to a toml file containing the build configs.") cmd.Flags().StringVarP(&o.SourceRepo, "source-repo", "s", "", "Required - URL of the source repo.") diff --git a/internal/builders/docker/pkg/common.go b/internal/builders/docker/pkg/common.go index f3acc4d437..03b60e36a4 100644 --- a/internal/builders/docker/pkg/common.go +++ b/internal/builders/docker/pkg/common.go @@ -85,6 +85,7 @@ type ArtifactReference struct { // OPTIONAL. LocalName string `json:"localName,omitempty"` + //nolint:lll // [URI] identifying the location that this artifact was downloaded from, if // different and not derivable from `uri`. // diff --git a/internal/builders/generic/attest.go b/internal/builders/generic/attest.go index a962e95ad4..51d20e6764 100644 --- a/internal/builders/generic/attest.go +++ b/internal/builders/generic/attest.go @@ -34,7 +34,9 @@ import ( ) // attestCmd returns the 'attest' command. -func attestCmd(provider slsa.ClientProvider, check func(error), signer signing.Signer, tlog signing.TransparencyLog) *cobra.Command { +func attestCmd(provider slsa.ClientProvider, check func(error), + signer signing.Signer, tlog signing.TransparencyLog, +) *cobra.Command { var attPath string var subjects string @@ -130,8 +132,14 @@ run in the context of a Github Actions workflow.`, }, } - c.Flags().StringVarP(&attPath, "signature", "g", "", "Path to write the signed provenance.") - c.Flags().StringVarP(&subjects, "subjects", "s", "", "Formatted list of subjects in the same format as sha256sum (base64 encoded).") + c.Flags().StringVarP( + &attPath, "signature", "g", "", + "Path to write the signed provenance.", + ) + c.Flags().StringVarP( + &subjects, "subjects", "s", "", + "Formatted list of subjects in the same format as sha256sum (base64 encoded).", + ) return c } diff --git a/internal/builders/generic/attest_test.go b/internal/builders/generic/attest_test.go index b92b485b9d..2ebcd078a7 100644 --- a/internal/builders/generic/attest_test.go +++ b/internal/builders/generic/attest_test.go @@ -18,6 +18,10 @@ import ( "github.com/slsa-framework/slsa-github-generator/slsa" ) +const ( + testHash = "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c artifact1" +) + // TestParseSubjects tests the parseSubjects function. func TestParseSubjects(t *testing.T) { testCases := []struct { @@ -54,8 +58,10 @@ func TestParseSubjects(t *testing.T) { }, { name: "extra whitespace", - // echo -e "\t 2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \t hoge fuga \t " | base64 -w0 - str: "CSAgMmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiAJIGhvZ2UgZnVnYSAgCSAgCg==", + // echo -e "\t 2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \ + // \t hoge fuga \t " | base64 -w0 + str: "CSAgMmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3Y" + + "mFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiAJIGhvZ2UgZnVnYSAgCSAgCg==", expected: []intoto.Subject{ { Name: "hoge fuga", @@ -68,8 +74,10 @@ func TestParseSubjects(t *testing.T) { { name: "multiple", - // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0 - str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCmU3MTJhZmYzNzA1YWMzMTRiOWE4OTBlMGVjMjA4ZmFhMjAwNTRlZWU1MTRkODZhYjkxM2Q3NjhmOTRlMDEyNzkgZnVnYQo=", + // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \ + // hoge\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0 + str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCmU" + + "3MTJhZmYzNzA1YWMzMTRiOWE4OTBlMGVjMjA4ZmFhMjAwNTRlZWU1MTRkODZhYjkxM2Q3NjhmOTRlMDEyNzkgZnVnYQo=", expected: []intoto.Subject{ { Name: "hoge", @@ -92,8 +100,10 @@ func TestParseSubjects(t *testing.T) { }, { name: "blank lines", - // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\n\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0 - str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCgplNzEyYWZmMzcwNWFjMzE0YjlhODkwZTBlYzIwOGZhYTIwMDU0ZWVlNTE0ZDg2YWI5MTNkNzY4Zjk0ZTAxMjc5IGZ1Z2EK", + // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \ + // hoge\n\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0 + str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCgp" + + "lNzEyYWZmMzcwNWFjMzE0YjlhODkwZTBlYzIwOGZhYTIwMDU0ZWVlNTE0ZDg2YWI5MTNkNzY4Zjk0ZTAxMjc5IGZ1Z2EK", expected: []intoto.Subject{ { Name: "hoge", @@ -123,8 +133,10 @@ func TestParseSubjects(t *testing.T) { }, { name: "duplicate name", - // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\n2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge" | base64 -w0 - str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCjJlMDM5MGViMDI0YTUyOTYzZGI3Yjk1ZTg0YTljMmIxMmMwMDQwNTRhN2JhZDlhOTdlYzBjN2M4OWQ0NjgxZDIgaG9nZQo=", + // echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \ + // hoge\n2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge" | base64 -w0 + str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dl" + + "CjJlMDM5MGViMDI0YTUyOTYzZGI3Yjk1ZTg0YTljMmIxMmMwMDQwNTRhN2JhZDlhOTdlYzBjN2M4OWQ0NjgxZDIgaG9nZQo=", err: &errDuplicateSubject{}, }, { @@ -175,7 +187,7 @@ func Test_attestCmd_default_single_artifact(t *testing.T) { c := attestCmd(&slsa.NilClientProvider{}, checkTest(t), &testutil.TestSigner{}, &testutil.TestTransparencyLog{}) c.SetOut(new(bytes.Buffer)) c.SetArgs([]string{ - "--subjects", base64.StdEncoding.EncodeToString([]byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c artifact1")), + "--subjects", base64.StdEncoding.EncodeToString([]byte(testHash)), }) if err := c.Execute(); err != nil { t.Errorf("unexpected failure: %v", err) @@ -243,7 +255,7 @@ func Test_attestCmd_custom_provenance_name(t *testing.T) { c := attestCmd(&slsa.NilClientProvider{}, checkTest(t), &testutil.TestSigner{}, &testutil.TestTransparencyLog{}) c.SetOut(new(bytes.Buffer)) c.SetArgs([]string{ - "--subjects", base64.StdEncoding.EncodeToString([]byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c artifact1")), + "--subjects", base64.StdEncoding.EncodeToString([]byte(testHash)), "--signature", "custom.intoto.jsonl", }) if err := c.Execute(); err != nil { @@ -289,7 +301,7 @@ func Test_attestCmd_invalid_extension(t *testing.T) { c := attestCmd(&slsa.NilClientProvider{}, check, &testutil.TestSigner{}, &testutil.TestTransparencyLog{}) c.SetOut(new(bytes.Buffer)) c.SetArgs([]string{ - "--subjects", base64.StdEncoding.EncodeToString([]byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c artifact1")), + "--subjects", base64.StdEncoding.EncodeToString([]byte(testHash)), "--signature", "invalid_name", }) if err := c.Execute(); err != nil { @@ -333,7 +345,7 @@ func Test_attestCmd_invalid_path(t *testing.T) { c := attestCmd(&slsa.NilClientProvider{}, check, &testutil.TestSigner{}, &testutil.TestTransparencyLog{}) c.SetOut(new(bytes.Buffer)) c.SetArgs([]string{ - "--subjects", base64.StdEncoding.EncodeToString([]byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c artifact1")), + "--subjects", base64.StdEncoding.EncodeToString([]byte(testHash)), "--signature", "/provenance.intoto.jsonl", }) if err := c.Execute(); err != nil { @@ -367,7 +379,7 @@ func Test_attestCmd_subdirectory_artifact(t *testing.T) { c := attestCmd(&slsa.NilClientProvider{}, checkTest(t), &testutil.TestSigner{}, &testutil.TestTransparencyLog{}) c.SetOut(new(bytes.Buffer)) c.SetArgs([]string{ - "--subjects", base64.StdEncoding.EncodeToString([]byte("b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c test/artifact1")), + "--subjects", base64.StdEncoding.EncodeToString([]byte(testHash)), }) if err := c.Execute(); err != nil { t.Errorf("unexpected failure: %v", err) diff --git a/internal/builders/go/pkg/build_test.go b/internal/builders/go/pkg/build_test.go index 30d4915616..3f46938b3c 100644 --- a/internal/builders/go/pkg/build_test.go +++ b/internal/builders/go/pkg/build_test.go @@ -814,7 +814,8 @@ func Test_generateLdflags(t *testing.T) { "start-{{ .Env.VAR3 }}-name-{{ .Env.VAR1 }}-end", "start-{{ .Env.VAR3 }}-name-{{ .Env.VAR2 }}-end", }, - outldflags: "start-value1-name-value2-end start-value1-name-value3-end start-value3-name-value1-end start-value3-name-value2-end", + outldflags: "start-value1-name-value2-end start-value1-name-value3-end " + + "start-value3-name-value1-end start-value3-name-value2-end", }, { name: "several ldflags and tag", @@ -828,7 +829,8 @@ func Test_generateLdflags(t *testing.T) { "{{ .Env.VAR3 }}-name-{{ .Env.VAR1 }}-{{ .Tag }}-{{ .Tag }}", "{{ .Env.VAR3 }}-name-{{ .Env.VAR2 }}-{{ .Tag }}-end", }, - outldflags: "start-value1-name-value2-v1.2.3-end value1-name-value3 value3-name-value1-v1.2.3-v1.2.3 value3-name-value2-v1.2.3-end", + outldflags: "start-value1-name-value2-v1.2.3-end value1-name-value3 " + + "value3-name-value1-v1.2.3-v1.2.3 value3-name-value2-v1.2.3-end", }, { name: "several ldflags and Arch and Os", diff --git a/internal/builders/go/pkg/provenance.go b/internal/builders/go/pkg/provenance.go index 1533862621..594894e907 100644 --- a/internal/builders/go/pkg/provenance.go +++ b/internal/builders/go/pkg/provenance.go @@ -64,7 +64,9 @@ func (b *goProvenanceBuild) BuildConfig(context.Context) (interface{}, error) { // GenerateProvenance translates github context into a SLSA provenance // attestation. // Spec: https://slsa.dev/provenance/v0.2 -func GenerateProvenance(name, digest, command, envs, workingDir string, s signing.Signer, r signing.TransparencyLog, provider slsa.ClientProvider) ([]byte, error) { +func GenerateProvenance(name, digest, command, envs, workingDir string, + s signing.Signer, r signing.TransparencyLog, provider slsa.ClientProvider, +) ([]byte, error) { gh, err := github.GetWorkflowContext() if err != nil { return nil, err @@ -159,7 +161,10 @@ func GenerateProvenance(name, digest, command, envs, workingDir string, s signin // Add details about the runner's OS to the materials runnerMaterials := slsacommon.ProvenanceMaterial{ // TODO: capture the digest here too - URI: fmt.Sprintf("https://github.com/actions/virtual-environments/releases/tag/%s/%s", os.Getenv("ImageOS"), os.Getenv("ImageVersion")), + URI: fmt.Sprintf( + "https://github.com/actions/virtual-environments/releases/tag/%s/%s", + os.Getenv("ImageOS"), os.Getenv("ImageVersion"), + ), } p.Predicate.Materials = append(p.Predicate.Materials, runnerMaterials) diff --git a/internal/builders/go/pkg/provenance_test.go b/internal/builders/go/pkg/provenance_test.go index b5784983ca..0c0aca5ae5 100644 --- a/internal/builders/go/pkg/provenance_test.go +++ b/internal/builders/go/pkg/provenance_test.go @@ -27,7 +27,11 @@ func TestGenerateProvenance_withErr(t *testing.T) { t.Setenv("GITHUB_EVENT_NAME", "non_event") t.Setenv("GITHUB_CONTEXT", "{}") sha256 := "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2" - _, err := GenerateProvenance("foo", sha256, "", "", "/home/foo", &testutil.TestSigner{}, &testutil.TransparencyLogWithErr{}, &slsa.NilClientProvider{}) + _, err := GenerateProvenance( + "foo", sha256, "", "", "/home/foo", + &testutil.TestSigner{}, &testutil.TransparencyLogWithErr{}, + &slsa.NilClientProvider{}, + ) if want, got := testutil.ErrTransparencyLog, err; want != got { t.Errorf("expected error, want: %v, got: %v", want, got) } diff --git a/internal/builders/nodejs/attest.go b/internal/builders/nodejs/attest.go index dbca44eab7..d1c0b2ed77 100644 --- a/internal/builders/nodejs/attest.go +++ b/internal/builders/nodejs/attest.go @@ -22,7 +22,9 @@ import ( ) // attestCmd runs the 'attest' command. -func attestCmd(provider slsa.ClientProvider, check func(error), signer signing.Signer, tlog signing.TransparencyLog) *cobra.Command { +func attestCmd(provider slsa.ClientProvider, check func(error), + signer signing.Signer, tlog signing.TransparencyLog, +) *cobra.Command { c := &cobra.Command{ Use: "attest", Short: "Run attest command", diff --git a/internal/utils/marshal_test.go b/internal/utils/marshal_test.go index 3001d46523..314abf71ee 100644 --- a/internal/utils/marshal_test.go +++ b/internal/utils/marshal_test.go @@ -41,7 +41,8 @@ func Test_MarshalToString(t *testing.T) { "-tags=netgo", "-ldflags=-X main.gitVersion=v1.2.3 -X main.gitSomething=somthg", }, - expected: "WyIvdXNyL2xpYi9nb29nbGUtZ29sYW5nL2Jpbi9nbyIsImJ1aWxkIiwiLW1vZD12ZW5kb3IiLCItdHJpbXBhdGgiLCItdGFncz1uZXRnbyIsIi1sZGZsYWdzPS1YIG1haW4uZ2l0VmVyc2lvbj12MS4yLjMgLVggbWFpbi5naXRTb21ldGhpbmc9c29tdGhnIl0=", + expected: "WyIvdXNyL2xpYi9nb29nbGUtZ29sYW5nL2Jpbi9nbyIsImJ1aWxkIiwiLW1vZD12ZW5kb3IiLCItdHJpbXBhdGgiLCItdGF" + + "ncz1uZXRnbyIsIi1sZGZsYWdzPS1YIG1haW4uZ2l0VmVyc2lvbj12MS4yLjMgLVggbWFpbi5naXRTb21ldGhpbmc9c29tdGhnIl0=", }, } for _, tt := range tests { @@ -85,7 +86,8 @@ func Test_UnmarshalList(t *testing.T) { "-tags=netgo", "-ldflags=-X main.gitVersion=v1.2.3 -X main.gitSomething=somthg", }, - value: "WyIvdXNyL2xpYi9nb29nbGUtZ29sYW5nL2Jpbi9nbyIsImJ1aWxkIiwiLW1vZD12ZW5kb3IiLCItdHJpbXBhdGgiLCItdGFncz1uZXRnbyIsIi1sZGZsYWdzPS1YIG1haW4uZ2l0VmVyc2lvbj12MS4yLjMgLVggbWFpbi5naXRTb21ldGhpbmc9c29tdGhnIl0=", + value: "WyIvdXNyL2xpYi9nb29nbGUtZ29sYW5nL2Jpbi9nbyIsImJ1aWxkIiwiLW1vZD12ZW5kb3IiLCItdHJpbXBhdGgiLCItdGFncz" + + "1uZXRnbyIsIi1sZGZsYWdzPS1YIG1haW4uZ2l0VmVyc2lvbj12MS4yLjMgLVggbWFpbi5naXRTb21ldGhpbmc9c29tdGhnIl0=", }, } for _, tt := range tests {