Fix support for --signature="" (slsa-framework#615)

Author: Ian Lewis

.github/workflows/generator_generic_slsa3.yml

@@ -114,6 +114,13 @@ jobs:
           UNTRUSTED_ATTESTATION_NAME: "${{ inputs.attestation-name }}"
         run: |
           set -euo pipefail
+          # NOTE: The generator binary allows the attestation to be "" in which
+          # case it does not sign or generate provenance. However, this workflow
+          # requires it to be non-empty so we validate it here.
+          if [ "$UNTRUSTED_ATTESTATION_NAME" == "" ]; then
+            echo "attestation-name cannot be empty."
+            exit 5
+          fi
           # Create and sign provenance.
           # Note: The builder verifies that the UNTRUSTED_ATTESTATION_NAME is located
           # in the current directory.

internal/builders/generic/attest.go

@@ -133,7 +133,7 @@ func (b *provenanceOnlyBuild) URI() string {
 }
 
 // attestCmd returns the 'attest' command.
-func attestCmd() *cobra.Command {
+func attestCmd(provider slsa.ClientProvider) *cobra.Command {
 	var predicatePath string
 	var attPath string
 	var subjects string
@@ -148,15 +148,13 @@ run in the context of a Github Actions workflow.`,
 		Run: func(cmd *cobra.Command, args []string) {
 			ghContext, err := github.GetWorkflowContext()
 			check(err)
-
-			// Verify the extension path and extension.
-			err = utils.VerifyAttestationPath(attPath)
-			check(err)
-
 			var parsedSubjects []intoto.Subject
 			// We don't actually care about the subjects if we aren't writing an attestation.
 			if attPath != "" {
-				var err error
+				// Verify the extension path and extension.
+				err = utils.VerifyAttestationPath(attPath)
+				check(err)
+
 				parsedSubjects, err = parseSubjects(subjects)
 				check(err)
 
@@ -170,15 +168,23 @@ run in the context of a Github Actions workflow.`,
 			b := provenanceOnlyBuild{
 				GithubActionsBuild: slsa.NewGithubActionsBuild(parsedSubjects, ghContext),
 			}
-			// TODO(github.com/slsa-framework/slsa-github-generator/issues/124): Remove
-			if utils.IsPresubmitTests() {
-				b.WithClients(&slsa.NilClientProvider{})
+			if provider != nil {
+				b.WithClients(provider)
+			} else {
+				// TODO(github.com/slsa-framework/slsa-github-generator/issues/124): Remove
+				if utils.IsPresubmitTests() {
+					b.WithClients(&slsa.NilClientProvider{})
+				}
 			}
 
 			g := slsa.NewHostedActionsGenerator(&b)
-			// TODO(github.com/slsa-framework/slsa-github-generator/issues/124): Remove
-			if utils.IsPresubmitTests() {
-				g.WithClients(&slsa.NilClientProvider{})
+			if provider != nil {
+				g.WithClients(provider)
+			} else {
+				// TODO(github.com/slsa-framework/slsa-github-generator/issues/124): Remove
+				if utils.IsPresubmitTests() {
+					g.WithClients(&slsa.NilClientProvider{})
+				}
 			}
 
 			p, err := g.Generate(ctx)

internal/builders/generic/attest_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bytes"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -9,6 +10,7 @@ import (
 	slsav02 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v0.2"
 
 	"github.com/slsa-framework/slsa-github-generator/internal/errors"
+	"github.com/slsa-framework/slsa-github-generator/slsa"
 )
 
 // TestParseSubjects tests the parseSubjects function.
@@ -145,3 +147,17 @@ func TestParseSubjects(t *testing.T) {
 		})
 	}
 }
+
+// Test_attestCmd tests the attest command.
+func Test_attestCmd(t *testing.T) {
+	t.Run("empty attestation path", func(t *testing.T) {
+		t.Setenv("GITHUB_CONTEXT", "{}")
+
+		c := attestCmd(&slsa.NilClientProvider{})
+		c.SetOut(new(bytes.Buffer))
+		c.SetArgs([]string{"--signature", ""})
+		if err := c.Execute(); err != nil {
+			t.Errorf("unexpected failure: %v", err)
+		}
+	})
+}

internal/builders/generic/main.go

@@ -44,7 +44,7 @@ For more information on SLSA, visit https://slsa.dev`,
 		},
 	}
 	c.AddCommand(versionCmd())
-	c.AddCommand(attestCmd())
+	c.AddCommand(attestCmd(nil))
 	return c
 }