From 6732aaaa8a33ea9f28ddf4060f81f5f9e0e71ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 13:30:37 +0200 Subject: [PATCH 01/10] refactor(attest): add an attestation submitter that posts to the attestation type URL --- cmd/kosli/attestationSubmit.go | 52 ++++++++++++++++++++++++ cmd/kosli/attestationSubmit_test.go | 62 +++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 cmd/kosli/attestationSubmit.go create mode 100644 cmd/kosli/attestationSubmit_test.go diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go new file mode 100644 index 000000000..423df58c7 --- /dev/null +++ b/cmd/kosli/attestationSubmit.go @@ -0,0 +1,52 @@ +package main + +import ( + "net/http" + "net/url" + + log "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/requests" +) + +// attestationSubmitter records Attestations on Trails. +type attestationSubmitter struct { + client *requests.Client + host string + org string + token string + dryRun bool + logger *log.Logger +} + +// attestationSubmission is one Attestation to record on a Trail. +type attestationSubmission struct { + flow string + trail string + // slug is the attestation type's segment in the attestations URL. + slug string + // label names the attestation type in the success message. + label string + payload any + attachments []string +} + +func (s *attestationSubmitter) submit(a attestationSubmission) error { + url, err := url.JoinPath(s.host, "api/v2/attestations", s.org, a.flow, "trail", a.trail, a.slug) + if err != nil { + return err + } + + form, _, _, err := newAttestationForm(a.payload, a.attachments) + if err != nil { + return err + } + + _, err = s.client.Do(&requests.RequestParams{ + Method: http.MethodPost, + URL: url, + Form: form, + DryRun: s.dryRun, + Token: s.token, + }) + return err +} diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go new file mode 100644 index 000000000..4e6770139 --- /dev/null +++ b/cmd/kosli/attestationSubmit_test.go @@ -0,0 +1,62 @@ +package main + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + + log "github.com/kosli-dev/cli/internal/logger" + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/require" +) + +type recordedAttestationRequest struct { + method string + path string +} + +func newFakeAttestationServer(t *testing.T) (*httptest.Server, *[]recordedAttestationRequest) { + t.Helper() + var recorded []recordedAttestationRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + recorded = append(recorded, recordedAttestationRequest{method: r.Method, path: r.URL.Path}) + w.WriteHeader(http.StatusCreated) + })) + t.Cleanup(server.Close) + return server, &recorded +} + +func newTestAttestationSubmitter(t *testing.T, host string, dryRun bool) (*attestationSubmitter, *bytes.Buffer) { + t.Helper() + var out bytes.Buffer + l := log.NewLogger(&out, &out, false) + client, err := requests.NewKosliClient("", 0, false, l) + require.NoError(t, err) + return &attestationSubmitter{ + client: client, + host: host, + org: "acme", + token: "secret", + dryRun: dryRun, + logger: l, + }, &out +} + +func TestAttestationSubmitterPostsToTheAttestationTypeURL(t *testing.T) { + server, recorded := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + + err := submitter.submit(attestationSubmission{ + flow: "my-flow", + trail: "my-trail", + slug: "generic", + label: "generic", + payload: &GenericAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{AttestationName: "unit-tests"}}, + }) + + require.NoError(t, err) + require.Len(t, *recorded, 1) + require.Equal(t, http.MethodPost, (*recorded)[0].method) + require.Equal(t, "/api/v2/attestations/acme/my-flow/trail/my-trail/generic", (*recorded)[0].path) +} From 641f18a8ad59211d1713762e3910ed04f5894a9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 13:31:54 +0200 Subject: [PATCH 02/10] refactor(attest): the attestation submitter uploads evidence and removes the tarball --- cmd/kosli/attestationSubmit.go | 10 ++- cmd/kosli/attestationSubmit_test.go | 128 ++++++++++++++++++++++++---- 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 423df58c7..810b140a1 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -3,6 +3,7 @@ package main import ( "net/http" "net/url" + "os" log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/requests" @@ -36,10 +37,17 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { return err } - form, _, _, err := newAttestationForm(a.payload, a.attachments) + form, cleanupNeeded, evidencePath, err := newAttestationForm(a.payload, a.attachments) if err != nil { return err } + if cleanupNeeded { + defer func() { + if err := os.Remove(evidencePath); err != nil { + s.logger.Warn("failed to remove evidence file: %v", err) + } + }() + } _, err = s.client.Do(&requests.RequestParams{ Method: http.MethodPost, diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index 4e6770139..3209f0025 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -2,8 +2,12 @@ package main import ( "bytes" + "encoding/json" + "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" log "github.com/kosli-dev/cli/internal/logger" @@ -12,19 +16,69 @@ import ( ) type recordedAttestationRequest struct { - method string - path string + method string + path string + dataJSON map[string]any + // files maps each file part's field name to its uploaded file name. + files map[string]string } -func newFakeAttestationServer(t *testing.T) (*httptest.Server, *[]recordedAttestationRequest) { +type fakeAttestationServer struct { + recorded []recordedAttestationRequest + // status and body answer every request; zero status means 201. + status int + body string +} + +func newFakeAttestationServer(t *testing.T) (*httptest.Server, *fakeAttestationServer) { t.Helper() - var recorded []recordedAttestationRequest + fake := &fakeAttestationServer{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - recorded = append(recorded, recordedAttestationRequest{method: r.Method, path: r.URL.Path}) - w.WriteHeader(http.StatusCreated) + require.NoError(t, r.ParseMultipartForm(1<<20)) + req := recordedAttestationRequest{method: r.Method, path: r.URL.Path, files: map[string]string{}} + require.NoError(t, json.Unmarshal([]byte(r.FormValue("data_json")), &req.dataJSON)) + for field, headers := range r.MultipartForm.File { + req.files[field] = headers[0].Filename + } + fake.recorded = append(fake.recorded, req) + status := fake.status + if status == 0 { + status = http.StatusCreated + } + w.WriteHeader(status) + _, _ = fmt.Fprint(w, fake.body) })) t.Cleanup(server.Close) - return server, &recorded + return server, fake +} + +func genericSubmission(name string) attestationSubmission { + return attestationSubmission{ + flow: "my-flow", + trail: "my-trail", + slug: "generic", + label: "generic", + payload: &GenericAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{AttestationName: name}}, + } +} + +// tarballsLeftIn lists evidence tarballs still on disk under tmp, where +// utils.Tar creates them when TMPDIR points at tmp. +func tarballsLeftIn(t *testing.T, tmp string) []string { + t.Helper() + left, err := filepath.Glob(filepath.Join(tmp, "*", "evidence.tgz")) + require.NoError(t, err) + return left +} + +func twoEvidenceFiles(t *testing.T) []string { + t.Helper() + dir := t.TempDir() + a := filepath.Join(dir, "a.txt") + b := filepath.Join(dir, "b.txt") + require.NoError(t, os.WriteFile(a, []byte("a"), 0o600)) + require.NoError(t, os.WriteFile(b, []byte("b"), 0o600)) + return []string{a, b} } func newTestAttestationSubmitter(t *testing.T, host string, dryRun bool) (*attestationSubmitter, *bytes.Buffer) { @@ -44,19 +98,57 @@ func newTestAttestationSubmitter(t *testing.T, host string, dryRun bool) (*attes } func TestAttestationSubmitterPostsToTheAttestationTypeURL(t *testing.T) { - server, recorded := newFakeAttestationServer(t) + server, fake := newFakeAttestationServer(t) submitter, _ := newTestAttestationSubmitter(t, server.URL, false) - err := submitter.submit(attestationSubmission{ - flow: "my-flow", - trail: "my-trail", - slug: "generic", - label: "generic", - payload: &GenericAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{AttestationName: "unit-tests"}}, - }) + err := submitter.submit(genericSubmission("unit-tests")) + + require.NoError(t, err) + require.Len(t, fake.recorded, 1) + require.Equal(t, http.MethodPost, fake.recorded[0].method) + require.Equal(t, "/api/v2/attestations/acme/my-flow/trail/my-trail/generic", fake.recorded[0].path) +} + +func TestAttestationSubmitterSendsThePayloadAsDataJSONWithoutAttachments(t *testing.T) { + server, fake := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + + err := submitter.submit(genericSubmission("unit-tests")) + + require.NoError(t, err) + require.Len(t, fake.recorded, 1) + require.Equal(t, "unit-tests", fake.recorded[0].dataJSON["attestation_name"]) + require.Empty(t, fake.recorded[0].files) +} + +func TestAttestationSubmitterUploadsAttachmentsAndRemovesTheTarball(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + server, fake := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.attachments = twoEvidenceFiles(t) + + err := submitter.submit(submission) require.NoError(t, err) - require.Len(t, *recorded, 1) - require.Equal(t, http.MethodPost, (*recorded)[0].method) - require.Equal(t, "/api/v2/attestations/acme/my-flow/trail/my-trail/generic", (*recorded)[0].path) + require.Len(t, fake.recorded, 1) + require.Equal(t, map[string]string{"attachment_file": "evidence.tgz"}, fake.recorded[0].files) + require.Empty(t, tarballsLeftIn(t, tmp)) +} + +func TestAttestationSubmitterRemovesTheTarballWhenTheServerRejects(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + server, fake := newFakeAttestationServer(t) + fake.status = http.StatusBadRequest + fake.body = `{"message":"rejected"}` + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.attachments = twoEvidenceFiles(t) + + err := submitter.submit(submission) + + require.EqualError(t, err, "rejected") + require.Empty(t, tarballsLeftIn(t, tmp)) } From 0ebf8f55b29e866761212a68d6f7d5752998c647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 13:32:37 +0200 Subject: [PATCH 03/10] refactor(attest): the attestation submitter logs success and explains binding errors --- cmd/kosli/attestation.go | 4 +++ cmd/kosli/attestationSubmit.go | 13 ++++++-- cmd/kosli/attestationSubmit_test.go | 47 +++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index 38731f0cf..ebe21ce1b 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -38,6 +38,10 @@ type CommonAttestationPayload struct { Annotations map[string]string `json:"annotations,omitempty"` } +func (p *CommonAttestationPayload) attestationName() string { + return p.AttestationName +} + type CommonAttestationOptions struct { fingerprintOptions *fingerprintOptions attestationNameTemplate string diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 810b140a1..165029518 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -27,10 +27,16 @@ type attestationSubmission struct { slug string // label names the attestation type in the success message. label string - payload any + payload attestationPayload attachments []string } +// attestationPayload is the body of any Attestation type; every one embeds +// CommonAttestationPayload, which provides the name. +type attestationPayload interface { + attestationName() string +} + func (s *attestationSubmitter) submit(a attestationSubmission) error { url, err := url.JoinPath(s.host, "api/v2/attestations", s.org, a.flow, "trail", a.trail, a.slug) if err != nil { @@ -56,5 +62,8 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { DryRun: s.dryRun, Token: s.token, }) - return err + if err == nil && !s.dryRun { + s.logger.Info("%s attestation '%s' is reported to trail: %s", a.label, a.payload.attestationName(), a.trail) + } + return wrapAttestationError(err) } diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index 3209f0025..78c9e8c8b 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -152,3 +152,50 @@ func TestAttestationSubmitterRemovesTheTarballWhenTheServerRejects(t *testing.T) require.EqualError(t, err, "rejected") require.Empty(t, tarballsLeftIn(t, tmp)) } + +func TestAttestationSubmitterLogsTheRecordedAttestation(t *testing.T) { + server, _ := newFakeAttestationServer(t) + submitter, out := newTestAttestationSubmitter(t, server.URL, false) + + err := submitter.submit(genericSubmission("unit-tests")) + + require.NoError(t, err) + require.Equal(t, "generic attestation 'unit-tests' is reported to trail: my-trail\n", out.String()) +} + +func TestAttestationSubmitterSendsNothingAndLogsNoSuccessInDryRun(t *testing.T) { + server, fake := newFakeAttestationServer(t) + submitter, out := newTestAttestationSubmitter(t, server.URL, true) + + err := submitter.submit(genericSubmission("unit-tests")) + + require.NoError(t, err) + require.Empty(t, fake.recorded) + require.Contains(t, out.String(), "THIS IS A DRY-RUN") + require.NotContains(t, out.String(), "is reported to trail") +} + +func TestAttestationSubmitterLogsNoSuccessWhenTheServerRejects(t *testing.T) { + server, fake := newFakeAttestationServer(t) + fake.status = http.StatusBadRequest + fake.body = `{"message":"rejected"}` + submitter, out := newTestAttestationSubmitter(t, server.URL, false) + + err := submitter.submit(genericSubmission("unit-tests")) + + require.EqualError(t, err, "rejected") + require.NotContains(t, out.String(), "is reported to trail") +} + +func TestAttestationSubmitterExplainsHowToBindTheAttestation(t *testing.T) { + server, fake := newFakeAttestationServer(t) + fake.status = http.StatusBadRequest + fake.body = `{"message":"Input payload requires at least one of: artifact_fingerprint or git_commit_info."}` + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + + err := submitter.submit(genericSubmission("unit-tests")) + + require.EqualError(t, err, "Input payload requires at least one of: specifying the fingerprint "+ + "(either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) "+ + "or providing --commit (requires an available git repo to access commit details)") +} From 99dca87c061f520e2ba6602e680e3ec7350a769c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:30:52 +0200 Subject: [PATCH 04/10] refactor(attest): attest generic records through the attestation submitter --- cmd/kosli/attestGeneric.go | 44 +++++++--------------------------- cmd/kosli/attestationSubmit.go | 11 +++++++++ 2 files changed, 20 insertions(+), 35 deletions(-) diff --git a/cmd/kosli/attestGeneric.go b/cmd/kosli/attestGeneric.go index c5b3781c6..7398beea9 100644 --- a/cmd/kosli/attestGeneric.go +++ b/cmd/kosli/attestGeneric.go @@ -3,11 +3,7 @@ package main import ( "fmt" "io" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -150,39 +146,17 @@ func newAttestGenericCmd(out io.Writer) *cobra.Command { } func (o *attestGenericOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "generic") + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) - if err != nil { - return err - } - - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file: %v", err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("generic attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "generic", + label: "generic", + payload: o.payload, + attachments: o.attachments, + }) } diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 165029518..246216434 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -19,6 +19,17 @@ type attestationSubmitter struct { logger *log.Logger } +func newAttestationSubmitter() *attestationSubmitter { + return &attestationSubmitter{ + client: kosliClient, + host: global.Host, + org: global.Org, + token: global.ApiToken, + dryRun: global.DryRun, + logger: logger, + } +} + // attestationSubmission is one Attestation to record on a Trail. type attestationSubmission struct { flow string From 93c519d9c5fe577287adfb9021e4b1a05e111413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:33:47 +0200 Subject: [PATCH 05/10] refactor(attest): custom, decision, junit and snyk record through the attestation submitter --- cmd/kosli/attestCustom.go | 44 ++++++++----------------------------- cmd/kosli/attestDecision.go | 43 ++++++++---------------------------- cmd/kosli/attestJunit.go | 44 ++++++++----------------------------- cmd/kosli/attestSnyk.go | 44 ++++++++----------------------------- 4 files changed, 36 insertions(+), 139 deletions(-) diff --git a/cmd/kosli/attestCustom.go b/cmd/kosli/attestCustom.go index d8f644245..c5d330701 100644 --- a/cmd/kosli/attestCustom.go +++ b/cmd/kosli/attestCustom.go @@ -3,11 +3,7 @@ package main import ( "fmt" "io" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -157,12 +153,7 @@ func newAttestCustomCmd(out io.Writer) *cobra.Command { } func (o *attestCustomOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "custom") - if err != nil { - return err - } - - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -172,29 +163,12 @@ func (o *attestCustomOptions) run(args []string) error { return fmt.Errorf("failed to load attestation data. %s", err) } - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file: %v", err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("custom:%s attestation '%s' is reported to trail: %s", o.payload.TypeName, o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "custom", + label: "custom:" + o.payload.TypeName, + payload: o.payload, + attachments: o.attachments, + }) } diff --git a/cmd/kosli/attestDecision.go b/cmd/kosli/attestDecision.go index 8c7918f47..98d3fea95 100644 --- a/cmd/kosli/attestDecision.go +++ b/cmd/kosli/attestDecision.go @@ -3,11 +3,7 @@ package main import ( "fmt" "io" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -153,38 +149,17 @@ func newAttestDecisionCmd(out io.Writer) *cobra.Command { } func (o *attestDecisionOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "system") + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) - if err != nil { - return err - } - - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file: %v", err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("decision attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "system", + label: "decision", + payload: o.payload, + attachments: o.attachments, + }) } diff --git a/cmd/kosli/attestJunit.go b/cmd/kosli/attestJunit.go index 1bc1c0318..f99504ab2 100644 --- a/cmd/kosli/attestJunit.go +++ b/cmd/kosli/attestJunit.go @@ -4,15 +4,11 @@ import ( "fmt" "io" "io/fs" - "net/http" - "net/url" - "os" "path/filepath" "strings" "time" junit "github.com/joshdk/go-junit" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -155,12 +151,7 @@ func newAttestJunitCmd(out io.Writer) *cobra.Command { } func (o *attestJunitOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "junit") - if err != nil { - return err - } - - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -175,31 +166,14 @@ func (o *attestJunitOptions) run(args []string) error { o.attachments = append(o.attachments, junitFilenames...) } - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file %s: %v", evidencePath, err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("junit attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "junit", + label: "junit", + payload: o.payload, + attachments: o.attachments, + }) } type JUnitResults struct { diff --git a/cmd/kosli/attestSnyk.go b/cmd/kosli/attestSnyk.go index 238aa2db1..d574cee28 100644 --- a/cmd/kosli/attestSnyk.go +++ b/cmd/kosli/attestSnyk.go @@ -3,11 +3,7 @@ package main import ( "fmt" "io" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/kosli-dev/cli/internal/snyk" "github.com/spf13/cobra" ) @@ -167,12 +163,7 @@ func newAttestSnykCmd(out io.Writer) *cobra.Command { } func (o *attestSnykOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "snyk") - if err != nil { - return err - } - - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -186,29 +177,12 @@ func (o *attestSnykOptions) run(args []string) error { o.attachments = append(o.attachments, o.snykSarifFilePath) } - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file %s: %v", evidencePath, err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("snyk attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "snyk", + label: "snyk", + payload: o.payload, + attachments: o.attachments, + }) } From 4e977484283257a20d27a6a38e6bff1ec70abeb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:34:28 +0200 Subject: [PATCH 06/10] refactor(attest): the attestation submitter reports assert failures after recording --- cmd/kosli/attestationSubmit.go | 13 ++++++++++ cmd/kosli/attestationSubmit_test.go | 38 +++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 246216434..532aa1262 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "net/http" "net/url" "os" @@ -40,6 +41,9 @@ type attestationSubmission struct { label string payload attestationPayload attachments []string + // assertFailures fail the command after the Attestation is recorded, so + // the evidence is kept even when --assert rejects it. + assertFailures []error } // attestationPayload is the body of any Attestation type; every one embeds @@ -76,5 +80,14 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { if err == nil && !s.dryRun { s.logger.Info("%s attestation '%s' is reported to trail: %s", a.label, a.payload.attestationName(), a.trail) } + if !s.dryRun { + for _, failure := range a.assertFailures { + if err != nil { + err = fmt.Errorf("%s\nError: %s", err.Error(), failure.Error()) + } else { + err = failure + } + } + } return wrapAttestationError(err) } diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index 78c9e8c8b..1cae700c4 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -3,6 +3,7 @@ package main import ( "bytes" "encoding/json" + "errors" "fmt" "net/http" "net/http/httptest" @@ -199,3 +200,40 @@ func TestAttestationSubmitterExplainsHowToBindTheAttestation(t *testing.T) { "(either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) "+ "or providing --commit (requires an available git repo to access commit details)") } + +func TestAttestationSubmitterReportsAssertFailuresAfterRecording(t *testing.T) { + server, fake := newFakeAttestationServer(t) + submitter, out := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.assertFailures = []error{errors.New("no references found"), errors.New("1 reference not found")} + + err := submitter.submit(submission) + + require.EqualError(t, err, "no references found\nError: 1 reference not found") + require.Len(t, fake.recorded, 1) + require.Equal(t, "generic attestation 'unit-tests' is reported to trail: my-trail\n", out.String()) +} + +func TestAttestationSubmitterJoinsAssertFailuresAfterTheServerError(t *testing.T) { + server, fake := newFakeAttestationServer(t) + fake.status = http.StatusBadRequest + fake.body = `{"message":"rejected"}` + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.assertFailures = []error{errors.New("no references found")} + + err := submitter.submit(submission) + + require.EqualError(t, err, "rejected\nError: no references found") +} + +func TestAttestationSubmitterDropsAssertFailuresInDryRun(t *testing.T) { + server, _ := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, true) + submission := genericSubmission("unit-tests") + submission.assertFailures = []error{errors.New("no references found")} + + err := submitter.submit(submission) + + require.NoError(t, err) +} From 63d8170ffbd111f19caa088e93bb4b3e527121ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:36:00 +0200 Subject: [PATCH 07/10] refactor(attest): sonar and jira record through the attestation submitter --- cmd/kosli/attestJira.go | 67 ++++++++++------------------------------ cmd/kosli/attestSonar.go | 46 ++++++--------------------- 2 files changed, 26 insertions(+), 87 deletions(-) diff --git a/cmd/kosli/attestJira.go b/cmd/kosli/attestJira.go index c3849f224..37bc5bcb5 100644 --- a/cmd/kosli/attestJira.go +++ b/cmd/kosli/attestJira.go @@ -3,9 +3,6 @@ package main import ( "fmt" "io" - "net/http" - "net/url" - "os" "regexp" "slices" "strings" @@ -13,7 +10,6 @@ import ( "github.com/kosli-dev/cli/internal/gitview" "github.com/kosli-dev/cli/internal/jira" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" ) @@ -325,13 +321,8 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command { } func (o *attestJiraOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "jira") - if err != nil { - return err - } - o.commitRequiredFor = "search for Jira issue keys" - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -418,53 +409,29 @@ func (o *attestJiraOptions) run(args []string) error { logger.Warn("%s", jiraUnconfirmedWarning(unconfirmedIDs, unconfirmedReasons)) } - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file: %v", err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, + var assertFailures []error + if o.assert && len(issueIDs) == 0 { + assertFailures = append(assertFailures, fmt.Errorf("no Jira references are found in %s", issueSource)) } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("jira attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - - if len(issueIDs) == 0 && o.assert && !global.DryRun { - errString := "" - if err != nil { - errString = fmt.Sprintf("%s\nError: ", err.Error()) - } - err = fmt.Errorf("%sno Jira references are found in %s", errString, issueSource) - } - - if issueFoundCount != len(issueIDs) && o.assert && !global.DryRun { - errString := "" - if err != nil { - errString = fmt.Sprintf("%s\nError: ", err.Error()) - } + if o.assert && issueFoundCount != len(issueIDs) { // prefixed, so it does not read as another entry in the issue list it follows reasonLog := "" for _, reason := range unconfirmedReasons { reasonLog += fmt.Sprintf("\n\treason: %s", reason) } - err = fmt.Errorf("%s%s from references found in %s%s%s", errString, - jiraAssertHeadline(len(issueIDs)-issueFoundCount-len(unconfirmedIDs), len(unconfirmedIDs)), issueSource, issueLog, reasonLog) + assertFailures = append(assertFailures, fmt.Errorf("%s from references found in %s%s%s", + jiraAssertHeadline(len(issueIDs)-issueFoundCount-len(unconfirmedIDs), len(unconfirmedIDs)), issueSource, issueLog, reasonLog)) } - return wrapAttestationError(err) + + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "jira", + label: "jira", + payload: o.payload, + attachments: o.attachments, + assertFailures: assertFailures, + }) } // jiraAssertHeadline names what went wrong in the first line of an --assert failure. A diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index 4fd2572cc..dbf4bb63a 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -2,11 +2,7 @@ package main import ( "io" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/kosli-dev/cli/internal/sonar" "github.com/spf13/cobra" ) @@ -245,12 +241,7 @@ func newAttestSonarCmd(out io.Writer) *cobra.Command { } func (o *attestSonarOptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "sonar") - if err != nil { - return err - } - - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -271,31 +262,12 @@ func (o *attestSonarOptions) run(args []string) error { return err } - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file %s: %v", evidencePath, err) - } - }() - } - - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("sonar attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - - return wrapAttestationError(err) - + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "sonar", + label: "sonar", + payload: o.payload, + attachments: o.attachments, + }) } From f466f3175d89ffcbf37367b32d8adf5d89824086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:37:55 +0200 Subject: [PATCH 08/10] refactor(attest): pull request attestations record through the attestation submitter --- cmd/kosli/pullrequest.go | 54 ++++++++++------------------------------ 1 file changed, 13 insertions(+), 41 deletions(-) diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index e78da1cf3..e02d6be0e 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -2,11 +2,7 @@ package main import ( "fmt" - "net/http" - "net/url" - "os" - "github.com/kosli-dev/cli/internal/requests" "github.com/kosli-dev/cli/internal/types" ) @@ -28,13 +24,8 @@ func (o *attestPROptions) getRetriever() types.PRRetriever { } func (o *attestPROptions) run(args []string) error { - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "pull_request") - if err != nil { - return err - } - o.commitRequiredFor = "find pull requests" - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -50,39 +41,20 @@ func (o *attestPROptions) run(args []string) error { o.payload.PullRequests = pullRequestsEvidence - form, cleanupNeeded, evidencePath, err := prepareAttestationForm(o.payload, o.attachments) - if err != nil { - return err - } - // if we created a tar package, remove it after uploading it - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - logger.Warn("failed to remove evidence file %s: %v", evidencePath, err) - } - }() - } - logger.Info("found %d %s(s) for commit: %s", len(pullRequestsEvidence), label, o.payload.Commit.Sha1) - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: form, - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("%s %s attestation '%s' is reported to trail: %s", o.payload.GitProvider, label, o.payload.AttestationName, o.trailName) - } - if len(pullRequestsEvidence) == 0 && o.assert && !global.DryRun { - errString := "" - if err != nil { - errString = fmt.Sprintf("%s\nError: ", err.Error()) - } - err = fmt.Errorf("%sassert failed: no %s found for the given commit: %s", errString, label, o.payload.Commit.Sha1) + var assertFailures []error + if o.assert && len(pullRequestsEvidence) == 0 { + assertFailures = append(assertFailures, fmt.Errorf("assert failed: no %s found for the given commit: %s", label, o.payload.Commit.Sha1)) } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + slug: "pull_request", + label: o.payload.GitProvider + " " + label, + payload: o.payload, + attachments: o.attachments, + assertFailures: assertFailures, + }) } From 2e665a570329a5feb79cf27cb06dc2114a574799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 14:41:25 +0200 Subject: [PATCH 09/10] refactor(attest): sbom records through the attestation submitter, which now owns evidence packaging --- cmd/kosli/attestSbom.go | 53 ++++------- cmd/kosli/attestSbom_test.go | 7 +- cmd/kosli/attestation.go | 42 --------- cmd/kosli/attestationSubmit.go | 133 ++++++++++++++++++++++++++-- cmd/kosli/attestationSubmit_test.go | 14 +++ cmd/kosli/cli_utils.go | 77 ---------------- 6 files changed, 159 insertions(+), 167 deletions(-) diff --git a/cmd/kosli/attestSbom.go b/cmd/kosli/attestSbom.go index 194b1a83a..06627e969 100644 --- a/cmd/kosli/attestSbom.go +++ b/cmd/kosli/attestSbom.go @@ -4,8 +4,6 @@ import ( "crypto/sha256" "fmt" "io" - "net/http" - "net/url" "os" "path/filepath" @@ -172,14 +170,7 @@ func newAttestSbomCmd(out io.Writer) *cobra.Command { } func (o *attestSbomOptions) run(args []string) error { - // The slug is the attestation family, not the type. The server tells them - // apart by type_name in the body. - url, err := url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "system") - if err != nil { - return err - } - - err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) + err := o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err } @@ -189,18 +180,16 @@ func (o *attestSbomOptions) run(args []string) error { return err } - reqParams := &requests.RequestParams{ - Method: http.MethodPost, - URL: url, - Form: o.attestationForm(content), - DryRun: global.DryRun, - Token: global.ApiToken, - } - _, err = kosliClient.Do(reqParams) - if err == nil && !global.DryRun { - logger.Info("sbom attestation '%s' is reported to trail: %s", o.payload.AttestationName, o.trailName) - } - return wrapAttestationError(err) + return newAttestationSubmitter().submit(attestationSubmission{ + flow: o.flowName, + trail: o.trailName, + // The slug is the attestation family, not the type. The server tells + // them apart by type_name in the body. + slug: "system", + label: "sbom", + payload: o.payload, + evidence: o.evidence(content), + }) } func (o *attestSbomOptions) loadSbom() ([]byte, error) { @@ -273,21 +262,17 @@ func (o *attestSbomOptions) rejectAttachments(cmd *cobra.Command) error { ) } -// attestationForm is the request body: the JSON payload and the SBOM bytes that -// were hashed, never a path. Handing the uploader a path would make it open and +// evidence is the attachment: the SBOM bytes that were hashed, never a path. Handing the uploader a path would make it open and // read the file a second time, and a file still being written would then be // uploaded as different bytes from the ones sbom_sha256 describes. // -// The bytes are a parameter rather than a field, so the body cannot be built -// before the file has been read. Built from an empty field, it would be a -// well-formed request carrying no attachment and a fingerprint of nothing. -func (o *attestSbomOptions) attestationForm(content []byte) []requests.FormItem { - return []requests.FormItem{ - {Type: "field", FieldName: "data_json", Content: o.payload}, - {Type: "file-bytes", FieldName: "attachment_file", Content: requests.FileBytes{ - Name: filepath.Base(o.sbomFilePath), - Data: content, - }}, +// The bytes are a parameter rather than a field, so the attachment cannot be +// built before the file has been read. Built from an empty field, it would be +// a well-formed request carrying no attachment and a fingerprint of nothing. +func (o *attestSbomOptions) evidence(content []byte) *requests.FileBytes { + return &requests.FileBytes{ + Name: filepath.Base(o.sbomFilePath), + Data: content, } } diff --git a/cmd/kosli/attestSbom_test.go b/cmd/kosli/attestSbom_test.go index b8a765b3d..e03e34fe0 100644 --- a/cmd/kosli/attestSbom_test.go +++ b/cmd/kosli/attestSbom_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/kosli-dev/cli/internal/requests" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -225,11 +224,7 @@ func TestSbomUploadsTheBytesItHashed(t *testing.T) { content, err := o.loadSbom() require.NoError(t, err) - form := o.attestationForm(content) - require.Len(t, form, 2, "the JSON payload and exactly one attachment") - require.Equal(t, "file-bytes", form[1].Type, "a path here would be read a second time by the uploader") - fb, ok := form[1].Content.(requests.FileBytes) - require.True(t, ok) + fb := o.evidence(content) require.Equal(t, "cyclonedx.json", fb.Name) uploaded := fmt.Sprintf("%x", sha256.Sum256(fb.Data)) diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index ebe21ce1b..8ec1707ae 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/kosli-dev/cli/internal/gitview" - "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/pflag" ) @@ -280,14 +279,6 @@ func processExternalURLs(externalURLs, externalFingerprints map[string]string) ( return processedExternalURLs, nil } -func prepareAttestationForm(payload any, evidencePaths []string) ([]requests.FormItem, bool, string, error) { - form, cleanupNeeded, evidencePath, err := newAttestationForm(payload, evidencePaths) - if err != nil { - return []requests.FormItem{}, cleanupNeeded, evidencePath, err - } - return form, cleanupNeeded, evidencePath, nil -} - func parseAttestationNameTemplate(template string) (string, string, error) { p1, p2, found := strings.Cut(template, ".") // No dot: treat the whole string as the attestation name @@ -301,39 +292,6 @@ func parseAttestationNameTemplate(template string) (string, string, error) { return p1, p2, nil } -// newAttestationForm constructs a list of FormItems for an attestation -// form submission. -func newAttestationForm(payload any, attachments []string) ( - []requests.FormItem, bool, string, error, -) { - form := []requests.FormItem{ - {Type: "field", FieldName: "data_json", Content: payload}, - } - - var evidencePath string - var cleanupNeeded bool - var err error - - if len(attachments) > 0 { - evidencePath, cleanupNeeded, err = getPathOfEvidenceFileToUpload(attachments) - if err != nil { - return form, cleanupNeeded, evidencePath, err - } - form = append(form, requests.FormItem{Type: "file", FieldName: "attachment_file", Content: evidencePath}) - logger.Debug("evidence file %s will be uploaded", evidencePath) - } - - return form, cleanupNeeded, evidencePath, nil -} - -func wrapAttestationError(err error) error { - if err != nil { - return fmt.Errorf("%s", strings.Replace(err.Error(), "requires at least one of: artifact_fingerprint or git_commit_info.", - "requires at least one of: specifying the fingerprint (either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) or providing --commit (requires an available git repo to access commit details)", 1)) - } - return err -} - func getGitRepoInfoFromEnvironment() (*gitview.GitRepoInfo, error) { ci := WhichCI() switch ci { diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 532aa1262..8d45d5fa8 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -5,9 +5,13 @@ import ( "net/http" "net/url" "os" + "path/filepath" + "strings" log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/requests" + "github.com/kosli-dev/cli/internal/utils" + cp "github.com/otiai10/copy" ) // attestationSubmitter records Attestations on Trails. @@ -41,6 +45,9 @@ type attestationSubmission struct { label string payload attestationPayload attachments []string + // evidence is an attachment already read into memory; it replaces + // attachments. + evidence *requests.FileBytes // assertFailures fail the command after the Attestation is recorded, so // the evidence is kept even when --assert rejects it. assertFailures []error @@ -58,17 +65,11 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { return err } - form, cleanupNeeded, evidencePath, err := newAttestationForm(a.payload, a.attachments) + form, cleanup, err := s.form(a) if err != nil { return err } - if cleanupNeeded { - defer func() { - if err := os.Remove(evidencePath); err != nil { - s.logger.Warn("failed to remove evidence file: %v", err) - } - }() - } + defer cleanup() _, err = s.client.Do(&requests.RequestParams{ Method: http.MethodPost, @@ -91,3 +92,119 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { } return wrapAttestationError(err) } + +// form returns the multipart form for a, and a cleanup that removes any +// evidence tarball the form refers to. +func (s *attestationSubmitter) form(a attestationSubmission) ([]requests.FormItem, func(), error) { + form := []requests.FormItem{ + {Type: "field", FieldName: "data_json", Content: a.payload}, + } + noCleanup := func() {} + + if a.evidence != nil { + return append(form, requests.FormItem{Type: "file-bytes", FieldName: "attachment_file", Content: *a.evidence}), noCleanup, nil + } + if len(a.attachments) == 0 { + return form, noCleanup, nil + } + + evidencePath, cleanupNeeded, err := getPathOfEvidenceFileToUpload(a.attachments) + if err != nil { + return nil, noCleanup, err + } + s.logger.Debug("evidence file %s will be uploaded", evidencePath) + form = append(form, requests.FormItem{Type: "file", FieldName: "attachment_file", Content: evidencePath}) + if !cleanupNeeded { + return form, noCleanup, nil + } + return form, func() { + if err := os.Remove(evidencePath); err != nil { + s.logger.Warn("failed to remove evidence file: %v", err) + } + }, nil +} + +// wrapAttestationError turns the server's binding error into one naming the +// flags that satisfy it. +func wrapAttestationError(err error) error { + if err != nil { + return fmt.Errorf("%s", strings.Replace(err.Error(), "requires at least one of: artifact_fingerprint or git_commit_info.", + "requires at least one of: specifying the fingerprint (either by calculating it using the artifact name/path and --artifact-type, or by providing it using --fingerprint) or providing --commit (requires an available git repo to access commit details)", 1)) + } + return err +} + +// getPathOfEvidenceFileToUpload returns the path of an evidence file to upload based +// on the provided evidencePaths. +// - if one path is provided and it is a file, that path is returned as it +// - if one path is provided and it is a directory, the directory is tarred and the +// path of the generated tar file is returned +// - if multiple paths are provided, they are packaged into a tar file and the +// path of the generated tar file is returned +func getPathOfEvidenceFileToUpload(evidencePaths []string) (string, bool, error) { + cleanupNeeded := false + if len(evidencePaths) == 0 { + return "", cleanupNeeded, fmt.Errorf("no evidence paths provided") + } + dirToTar := "" + if len(evidencePaths) == 1 { + ok, err := utils.IsFile(evidencePaths[0]) + if err != nil { + return "", cleanupNeeded, err + } + if ok { + logger.Debug("file %s is provided as evidence", evidencePaths[0]) + return evidencePaths[0], cleanupNeeded, nil + } + + ok, err = utils.IsDir(evidencePaths[0]) + if err != nil { + return "", cleanupNeeded, err + } + if ok { + logger.Debug("dir %s is provided as evidence. It will be tarred", evidencePaths[0]) + dirToTar = evidencePaths[0] + } + + } else { // there are multiple paths + // copy all paths to a new temp dir + tmpDir, err := os.MkdirTemp("", "") + if err != nil { + return "", cleanupNeeded, err + } + + logger.Debug("[%d] paths are provided as evidence. They will be tarred from %s", len(evidencePaths), tmpDir) + + for _, path := range evidencePaths { + volume := filepath.VolumeName(path) + volumeWithoutColon := strings.Replace(volume, ":", "", 1) + pathWithoutVolume := path[len(volume):] + pathWithoutColon := volumeWithoutColon + pathWithoutVolume + + // this copy is only a staging area for the tarball built below, and + // nothing downstream reads uid/gid. Preserving ownership would Lchown + // to a foreign uid, which fails for any non-root process handed + // evidence written by a docker container. + err := cp.Copy(path, filepath.Join(tmpDir, pathWithoutColon), cp.Options{ + PreserveTimes: true, + }) + if err != nil { + return "", cleanupNeeded, fmt.Errorf("failed to package attachment %s: %w", path, err) + } + } + dirToTar = tmpDir + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + logger.Warn("failed to remove temporary directory %s: %v", tmpDir, err) + } + }() + } + + // tar the required dir and return the path of the tar file + tarFilePath, err := utils.Tar(dirToTar, "evidence.tgz") + if err != nil { + return "", cleanupNeeded, err + } + cleanupNeeded = true + return tarFilePath, cleanupNeeded, nil +} diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index 1cae700c4..a23d244f8 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -237,3 +237,17 @@ func TestAttestationSubmitterDropsAssertFailuresInDryRun(t *testing.T) { require.NoError(t, err) } + +func TestAttestationSubmitterUploadsEvidenceAlreadyInMemory(t *testing.T) { + server, fake := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.evidence = &requests.FileBytes{Name: "sbom.json", Data: []byte(`{"bomFormat":"CycloneDX"}`)} + + err := submitter.submit(submission) + + require.NoError(t, err) + require.Len(t, fake.recorded, 1) + require.Equal(t, "unit-tests", fake.recorded[0].dataJSON["attestation_name"]) + require.Equal(t, map[string]string{"attachment_file": "sbom.json"}, fake.recorded[0].files) +} diff --git a/cmd/kosli/cli_utils.go b/cmd/kosli/cli_utils.go index a5c58ddcc..c14ea6903 100644 --- a/cmd/kosli/cli_utils.go +++ b/cmd/kosli/cli_utils.go @@ -7,7 +7,6 @@ import ( "io" urlPackage "net/url" "os" - "path/filepath" "reflect" "slices" "strconv" @@ -20,7 +19,6 @@ import ( "github.com/kosli-dev/cli/internal/gitview" log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/utils" - cp "github.com/otiai10/copy" "github.com/spf13/cobra" "github.com/xeonx/timeago" "golang.org/x/term" @@ -622,81 +620,6 @@ func formattedTimestamp(timestamp any, short bool) (string, error) { } } -// getPathOfEvidenceFileToUpload returns the path of an evidence file to upload based -// on the provided evidencePaths. -// - if one path is provided and it is a file, that path is returned as it -// - if one path is provided and it is a directory, the directory is tarred and the -// path of the generated tar file is returned -// - if multiple paths are provided, they are packaged into a tar file and the -// path of the generated tar file is returned -func getPathOfEvidenceFileToUpload(evidencePaths []string) (string, bool, error) { - cleanupNeeded := false - if len(evidencePaths) == 0 { - return "", cleanupNeeded, fmt.Errorf("no evidence paths provided") - } - dirToTar := "" - if len(evidencePaths) == 1 { - ok, err := utils.IsFile(evidencePaths[0]) - if err != nil { - return "", cleanupNeeded, err - } - if ok { - logger.Debug("file %s is provided as evidence", evidencePaths[0]) - return evidencePaths[0], cleanupNeeded, nil - } - - ok, err = utils.IsDir(evidencePaths[0]) - if err != nil { - return "", cleanupNeeded, err - } - if ok { - logger.Debug("dir %s is provided as evidence. It will be tarred", evidencePaths[0]) - dirToTar = evidencePaths[0] - } - - } else { // there are multiple paths - // copy all paths to a new temp dir - tmpDir, err := os.MkdirTemp("", "") - if err != nil { - return "", cleanupNeeded, err - } - - logger.Debug("[%d] paths are provided as evidence. They will be tarred from %s", len(evidencePaths), tmpDir) - - for _, path := range evidencePaths { - volume := filepath.VolumeName(path) - volumeWithoutColon := strings.Replace(volume, ":", "", 1) - pathWithoutVolume := path[len(volume):] - pathWithoutColon := volumeWithoutColon + pathWithoutVolume - - // this copy is only a staging area for the tarball built below, and - // nothing downstream reads uid/gid. Preserving ownership would Lchown - // to a foreign uid, which fails for any non-root process handed - // evidence written by a docker container. - err := cp.Copy(path, filepath.Join(tmpDir, pathWithoutColon), cp.Options{ - PreserveTimes: true, - }) - if err != nil { - return "", cleanupNeeded, fmt.Errorf("failed to package attachment %s: %w", path, err) - } - } - dirToTar = tmpDir - defer func() { - if err := os.RemoveAll(tmpDir); err != nil { - logger.Warn("failed to remove temporary directory %s: %v", tmpDir, err) - } - }() - } - - // tar the required dir and return the path of the tar file - tarFilePath, err := utils.Tar(dirToTar, "evidence.tgz") - if err != nil { - return "", cleanupNeeded, err - } - cleanupNeeded = true - return tarFilePath, cleanupNeeded, nil -} - // handleExpressions parses ~ and # expressions and returns // a name (usually flow name), an ID (positive for fixed IDs or negative for reverse IDs), // and an error if the expression is invalid From 2595236793aba9a38d6c0dfe362ad1ae31fe49f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 2 Oct 2026 15:43:04 +0200 Subject: [PATCH 10/10] fix(attest): remove the evidence staging dir when an attachment fails to copy The staging dir's cleanup was registered only after every attachment had been copied, so a missing or unreadable attachment left it in $TMPDIR. Also from review on #1243: - getPathOfEvidenceFileToUpload takes the submitter's logger - a submission with both in-memory evidence and attachments is rejected instead of silently dropping the attachments - the fake attestation server reports failures with assert and a 500, since require's FailNow is unsafe off the test goroutine - the endpoint variable no longer shadows net/url --- cmd/kosli/attestationSubmit.go | 25 ++++++++-------- cmd/kosli/attestationSubmit_test.go | 44 +++++++++++++++++++++++++++-- cmd/kosli/cli_utils_test.go | 6 ++-- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/cmd/kosli/attestationSubmit.go b/cmd/kosli/attestationSubmit.go index 8d45d5fa8..f3d90f4ca 100644 --- a/cmd/kosli/attestationSubmit.go +++ b/cmd/kosli/attestationSubmit.go @@ -45,8 +45,8 @@ type attestationSubmission struct { label string payload attestationPayload attachments []string - // evidence is an attachment already read into memory; it replaces - // attachments. + // evidence is an attachment already read into memory; it cannot be + // combined with attachments. evidence *requests.FileBytes // assertFailures fail the command after the Attestation is recorded, so // the evidence is kept even when --assert rejects it. @@ -60,7 +60,7 @@ type attestationPayload interface { } func (s *attestationSubmitter) submit(a attestationSubmission) error { - url, err := url.JoinPath(s.host, "api/v2/attestations", s.org, a.flow, "trail", a.trail, a.slug) + endpoint, err := url.JoinPath(s.host, "api/v2/attestations", s.org, a.flow, "trail", a.trail, a.slug) if err != nil { return err } @@ -73,7 +73,7 @@ func (s *attestationSubmitter) submit(a attestationSubmission) error { _, err = s.client.Do(&requests.RequestParams{ Method: http.MethodPost, - URL: url, + URL: endpoint, Form: form, DryRun: s.dryRun, Token: s.token, @@ -101,6 +101,9 @@ func (s *attestationSubmitter) form(a attestationSubmission) ([]requests.FormIte } noCleanup := func() {} + if a.evidence != nil && len(a.attachments) > 0 { + return nil, noCleanup, fmt.Errorf("an attestation carries either in-memory evidence or attachments, not both") + } if a.evidence != nil { return append(form, requests.FormItem{Type: "file-bytes", FieldName: "attachment_file", Content: *a.evidence}), noCleanup, nil } @@ -108,7 +111,7 @@ func (s *attestationSubmitter) form(a attestationSubmission) ([]requests.FormIte return form, noCleanup, nil } - evidencePath, cleanupNeeded, err := getPathOfEvidenceFileToUpload(a.attachments) + evidencePath, cleanupNeeded, err := getPathOfEvidenceFileToUpload(a.attachments, s.logger) if err != nil { return nil, noCleanup, err } @@ -141,7 +144,7 @@ func wrapAttestationError(err error) error { // path of the generated tar file is returned // - if multiple paths are provided, they are packaged into a tar file and the // path of the generated tar file is returned -func getPathOfEvidenceFileToUpload(evidencePaths []string) (string, bool, error) { +func getPathOfEvidenceFileToUpload(evidencePaths []string, logger *log.Logger) (string, bool, error) { cleanupNeeded := false if len(evidencePaths) == 0 { return "", cleanupNeeded, fmt.Errorf("no evidence paths provided") @@ -172,6 +175,11 @@ func getPathOfEvidenceFileToUpload(evidencePaths []string) (string, bool, error) if err != nil { return "", cleanupNeeded, err } + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + logger.Warn("failed to remove temporary directory %s: %v", tmpDir, err) + } + }() logger.Debug("[%d] paths are provided as evidence. They will be tarred from %s", len(evidencePaths), tmpDir) @@ -193,11 +201,6 @@ func getPathOfEvidenceFileToUpload(evidencePaths []string) (string, bool, error) } } dirToTar = tmpDir - defer func() { - if err := os.RemoveAll(tmpDir); err != nil { - logger.Warn("failed to remove temporary directory %s: %v", tmpDir, err) - } - }() } // tar the required dir and return the path of the tar file diff --git a/cmd/kosli/attestationSubmit_test.go b/cmd/kosli/attestationSubmit_test.go index a23d244f8..95aa43ef4 100644 --- a/cmd/kosli/attestationSubmit_test.go +++ b/cmd/kosli/attestationSubmit_test.go @@ -13,6 +13,7 @@ import ( log "github.com/kosli-dev/cli/internal/logger" "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -35,9 +36,17 @@ func newFakeAttestationServer(t *testing.T) (*httptest.Server, *fakeAttestationS t.Helper() fake := &fakeAttestationServer{} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - require.NoError(t, r.ParseMultipartForm(1<<20)) + // The handler runs off the test goroutine, where require's FailNow + // is unsafe, so failures are reported with assert and answered. + if !assert.NoError(t, r.ParseMultipartForm(1<<20)) { + http.Error(w, "unparseable form", http.StatusInternalServerError) + return + } req := recordedAttestationRequest{method: r.Method, path: r.URL.Path, files: map[string]string{}} - require.NoError(t, json.Unmarshal([]byte(r.FormValue("data_json")), &req.dataJSON)) + if !assert.NoError(t, json.Unmarshal([]byte(r.FormValue("data_json")), &req.dataJSON)) { + http.Error(w, "unparseable data_json", http.StatusInternalServerError) + return + } for field, headers := range r.MultipartForm.File { req.files[field] = headers[0].Filename } @@ -251,3 +260,34 @@ func TestAttestationSubmitterUploadsEvidenceAlreadyInMemory(t *testing.T) { require.Equal(t, "unit-tests", fake.recorded[0].dataJSON["attestation_name"]) require.Equal(t, map[string]string{"attachment_file": "sbom.json"}, fake.recorded[0].files) } + +func TestAttestationSubmitterRemovesTheStagingDirWhenAnAttachmentIsMissing(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + server, fake := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + attachments := twoEvidenceFiles(t) + submission.attachments = []string{attachments[0], filepath.Join(filepath.Dir(attachments[0]), "missing.txt")} + + err := submitter.submit(submission) + + require.ErrorContains(t, err, "missing.txt") + require.Empty(t, fake.recorded) + left, readErr := os.ReadDir(tmp) + require.NoError(t, readErr) + require.Empty(t, left, "the staging directory must not outlive a failed copy") +} + +func TestAttestationSubmitterRejectsEvidenceTogetherWithAttachments(t *testing.T) { + server, fake := newFakeAttestationServer(t) + submitter, _ := newTestAttestationSubmitter(t, server.URL, false) + submission := genericSubmission("unit-tests") + submission.evidence = &requests.FileBytes{Name: "sbom.json", Data: []byte(`{}`)} + submission.attachments = twoEvidenceFiles(t) + + err := submitter.submit(submission) + + require.Error(t, err) + require.Empty(t, fake.recorded) +} diff --git a/cmd/kosli/cli_utils_test.go b/cmd/kosli/cli_utils_test.go index cf50a27e9..280f70e0a 100644 --- a/cmd/kosli/cli_utils_test.go +++ b/cmd/kosli/cli_utils_test.go @@ -1259,7 +1259,7 @@ func (suite *CliUtilsTestSuite) TestGetPathOfEvidenceFileToUploadMultiplePaths() root := suite.T().TempDir() dir, file := evidenceFixture(suite.T(), root) - tarPath, cleanupNeeded, err := getPathOfEvidenceFileToUpload([]string{dir, file}) + tarPath, cleanupNeeded, err := getPathOfEvidenceFileToUpload([]string{dir, file}, logger) require.NoError(suite.T(), err) require.True(suite.T(), cleanupNeeded, "a generated tarball must be cleaned up by the caller") defer func() { _ = os.RemoveAll(filepath.Dir(tarPath)) }() @@ -1296,7 +1296,7 @@ func (suite *CliUtilsTestSuite) TestGetPathOfEvidenceFileToUploadDoesNotPreserve require.NoError(suite.T(), os.Chown(filepath.Join(dir, "junit.xml"), foreignUID, foreignGID), "the fixture stands in for evidence written by a docker container") - tarPath, _, err := getPathOfEvidenceFileToUpload([]string{dir, file}) + tarPath, _, err := getPathOfEvidenceFileToUpload([]string{dir, file}, logger) require.NoError(suite.T(), err, "evidence owned by another user must still be packaged") defer func() { _ = os.RemoveAll(filepath.Dir(tarPath)) }() @@ -1318,7 +1318,7 @@ func (suite *CliUtilsTestSuite) TestGetPathOfEvidenceFileToUploadNamesTheFailing _, file := evidenceFixture(suite.T(), root) missing := filepath.Join(root, "does-not-exist") - _, _, err := getPathOfEvidenceFileToUpload([]string{file, missing}) + _, _, err := getPathOfEvidenceFileToUpload([]string{file, missing}, logger) require.Error(suite.T(), err) require.Contains(suite.T(), err.Error(), missing,