diff --git a/cmd/kosli/attest.go b/cmd/kosli/attest.go index 1632455bf..90af8ba9f 100644 --- a/cmd/kosli/attest.go +++ b/cmd/kosli/attest.go @@ -25,6 +25,7 @@ func newAttestCmd(out io.Writer) *cobra.Command { newAttestPRCmd(out), newAttestSonarCmd(out), newAttestCustomCmd(out), + newAttestSbomCmd(out), newAttestOverrideCmd(out), newAttestDecisionCmd(out), ) diff --git a/cmd/kosli/attestSbom.go b/cmd/kosli/attestSbom.go new file mode 100644 index 000000000..a90a2686a --- /dev/null +++ b/cmd/kosli/attestSbom.go @@ -0,0 +1,328 @@ +package main + +import ( + "crypto/sha256" + "fmt" + "io" + "net/http" + "net/url" + "os" + "path/filepath" + + "github.com/kosli-dev/cli/internal/requests" + "github.com/kosli-dev/cli/internal/sbom" + "github.com/spf13/cobra" +) + +// The API rejects a request body over 10MB, and the JSON payload is counted +// alongside the file, so this leaves room for it rather than sitting on the +// limit. The margin is a guess at a small payload, not a bound: --user-data +// embeds an arbitrary JSON file in the same body and can push the total past +// 10MB on its own. Lifting the ceiling needs direct-to-S3 upload, tracked +// separately. +const maxSbomFileBytes = 9 * 1024 * 1024 + +// Both are also carried inside attestation_data, where the server's schema can +// enforce them. These are the copy a reader sees on the trail page. +const ( + sbomFormatAnnotation = "sbom_format" + sbomSha256Annotation = "sbom_sha256" +) + +type SbomAttestationData struct { + Format string `json:"format"` + OriginalFingerprint string `json:"original_fingerprint"` + Document *sbom.Document `json:"document"` +} + +type SbomAttestationPayload struct { + *CommonAttestationPayload + TypeName string `json:"type_name"` + AttestationData SbomAttestationData `json:"attestation_data"` +} + +type attestSbomOptions struct { + *CommonAttestationOptions + sbomFilePath string + payload SbomAttestationPayload +} + +const attestSbomShortDesc = `Report a software bill of materials to an artifact or a trail in a Kosli flow. ` + +const attestSbomLongDesc = attestSbomShortDesc + ` +The SBOM file is given with the ^--sbom-file^ flag. CycloneDX (JSON and XML) and +SPDX (JSON and tag-value) are supported. + +The file is uploaded as it is, so the recorded checksum is the checksum of the +file you supplied and you can verify it by hand. It must be a single file: it is +not compressed, and a gzipped file is rejected, because the format and the +summary below are read from it. + +Kosli reads the format, the creation time, the tools that produced it, the +subject it describes and how many packages it lists. Nothing is checked against +the artifact; the SBOM is recorded as reported. + +The SBOM file is the only attachment: this command does not accept additional +attachments, because two or more would be compressed together. + +The format and the file checksum are also added as the ^sbom_format^ and +^sbom_sha256^ annotations. +` + attestationBindingDesc + ` + +` + kosliIgnoreDesc + ` + +` + commitDescription + +const attestSbomExample = ` +# report an SBOM about a pre-built docker artifact (kosli finds the fingerprint): +kosli attest sbom yourDockerImageName \ + --artifact-type docker \ + --name yourAttestationName \ + --sbom-file yourSbomPath \ + --flow yourFlowName \ + --trail yourTrailName \ + --api-token yourAPIToken \ + --org yourOrgName + +# report an SBOM about a trail: +kosli attest sbom \ + --name yourAttestationName \ + --sbom-file yourSbomPath \ + --flow yourFlowName \ + --trail yourTrailName \ + --api-token yourAPIToken \ + --org yourOrgName +` + +func newAttestSbomCmd(out io.Writer) *cobra.Command { + o := &attestSbomOptions{ + CommonAttestationOptions: &CommonAttestationOptions{ + fingerprintOptions: &fingerprintOptions{}, + }, + payload: SbomAttestationPayload{ + CommonAttestationPayload: &CommonAttestationPayload{}, + TypeName: "sbom", + }, + } + cmd := &cobra.Command{ + // Args: cobra.MaximumNArgs(1), // See CustomMaximumNArgs() below + Use: "sbom [IMAGE-NAME | FILE-PATH | DIR-PATH]", + Short: attestSbomShortDesc, + Long: attestSbomLongDesc, + Example: attestSbomExample, + Annotations: map[string]string{betaCLIAnnotation: ""}, + PreRunE: func(cmd *cobra.Command, args []string) error { + err := CustomMaximumNArgs(1, args) + if err != nil { + return err + } + + err = RequireGlobalFlags(global, []string{"Org", "ApiToken"}) + if err != nil { + return ErrorBeforePrintingUsage(cmd, err.Error()) + } + + err = MuXRequiredFlags(cmd, []string{"fingerprint", "artifact-type"}, false) + if err != nil { + return err + } + + err = o.rejectAttachments(cmd) + if err != nil { + return err + } + + err = o.rejectReservedAnnotations() + if err != nil { + return err + } + + err = ValidateSliceValues(o.redactedCommitInfo, allowedCommitRedactionValues) + if err != nil { + return fmt.Errorf("%s for --redact-commit-info", err.Error()) + } + + err = ValidateAttestationArtifactArg(args, o.fingerprintOptions.artifactType, o.payload.ArtifactFingerprint) + if err != nil { + return ErrorBeforePrintingUsage(cmd, err.Error()) + } + + return ValidateRegistryFlags(cmd, o.fingerprintOptions) + }, + + RunE: func(cmd *cobra.Command, args []string) error { + o.repoURLExplicit = cmd.Flags().Changed("repo-url") + o.repoNameExplicit = cmd.Flags().Changed("repository") + return o.run(args) + }, + } + + ci := WhichCI() + addAttestationFlags(cmd, o.CommonAttestationOptions, o.payload.CommonAttestationPayload, ci) + cmd.Flags().StringVar(&o.sbomFilePath, "sbom-file", "", attestationSbomFileFlag) + // Required --sbom-file and mutually exclusive --attachments can never both be + // satisfied, so the flag is hidden from help. Passing it still gets the error. + _ = cmd.Flags().MarkHidden("attachments") + + err := RequireFlags(cmd, []string{"flow", "trail", "name", "sbom-file"}) + if err != nil { + logger.Error("failed to configure required flags: %v", err) + } + + return cmd +} + +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) + if err != nil { + return err + } + + content, err := o.loadSbom() + if err != nil { + 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) +} + +func (o *attestSbomOptions) loadSbom() ([]byte, error) { + // Checked before opening. open(2) on a fifo with no writer blocks, so a + // check after it would never run; and a directory is what a user hits when + // tab-completion stops a path short, so it gets its own advice. + info, err := os.Stat(o.sbomFilePath) + if err != nil { + return nil, fmt.Errorf("failed to read SBOM file [%s]: %s", o.sbomFilePath, err) + } + if info.IsDir() { + return nil, fmt.Errorf("SBOM file [%s] is a directory; supply the SBOM file itself", o.sbomFilePath) + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("SBOM file [%s] is not a regular file", o.sbomFilePath) + } + + file, err := os.Open(o.sbomFilePath) + if err != nil { + return nil, fmt.Errorf("failed to read SBOM file [%s]: %s", o.sbomFilePath, err) + } + defer func() { _ = file.Close() }() + + // One read serves the fingerprint, the summary and the upload, so the + // recorded checksum describes the bytes the server receives even if the file + // is still being written. Reading one byte past the limit is what makes the + // limit a bound rather than a claim about a size measured earlier. + content, err := io.ReadAll(io.LimitReader(file, maxSbomFileBytes+1)) + if err != nil { + return nil, fmt.Errorf("failed to read SBOM file [%s]: %s", o.sbomFilePath, err) + } + if int64(len(content)) > maxSbomFileBytes { + return nil, fmt.Errorf( + "SBOM file [%s] is above the %d byte limit for an SBOM attestation", + o.sbomFilePath, maxSbomFileBytes, + ) + } + fingerprint := fmt.Sprintf("%x", sha256.Sum256(content)) + + data, err := sbom.ProcessSBOM(content) + if err != nil { + return nil, fmt.Errorf("failed to parse SBOM file [%s]: %s", o.sbomFilePath, err) + } + + o.payload.AttestationData = SbomAttestationData{ + Format: data.Format, + OriginalFingerprint: fingerprint, + Document: data.Document, + } + o.annotate(data.Format, fingerprint) + return content, nil +} + +// rejectAttachments refuses --attachments however it was set. Two or more +// attachments are tarred and gzipped before upload, which would compress the +// SBOM and break the checksum recorded against it. The flag is hidden from help, +// and a value from KOSLI_ATTACHMENTS or a config file marks it Changed without +// anyone typing it, so in that case the message says where the value came from. +func (o *attestSbomOptions) rejectAttachments(cmd *cobra.Command) error { + if !cmd.Flags().Changed("attachments") { + return nil + } + source := "" + if flagCameFromConfig("attachments") { + source = " (set by " + configValueSource("attachments") + ")" + } + return fmt.Errorf( + "--attachments cannot be used with attest sbom%s: the SBOM file is the only attachment, and a second one would be compressed", + source, + ) +} + +// 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 +// 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, + }}, + } +} + +// rejectReservedAnnotations runs in PreRunE: it needs nothing from the file, so +// a typo should not cost a repository walk and a pass over nine megabytes first. +func (o *attestSbomOptions) rejectReservedAnnotations() error { + for _, reserved := range []string{sbomFormatAnnotation, sbomSha256Annotation} { + if _, taken := o.annotations[reserved]; taken { + return fmt.Errorf( + "annotation key '%s' is set by this command from the SBOM file and cannot be provided with --annotate", + reserved, + ) + } + } + return nil +} + +// annotate records what the file said about itself where a reader sees it on +// the trail page. The same two values are carried inside attestation_data. +// +// It must run after CommonAttestationOptions.run, which assigns +// payload.Annotations wholesale from the --annotate flag. Called before that, +// both keys are discarded. +// +// It builds a new map rather than writing into that one. The two are the same +// map -- processAnnotations returns its argument -- so writing would put these +// keys into the --annotate map that rejectReservedAnnotations reads, and a +// second run of the same options would be refused for a key nobody supplied. +func (o *attestSbomOptions) annotate(format, fingerprint string) { + merged := make(map[string]string, len(o.payload.Annotations)+2) + for key, value := range o.payload.Annotations { + merged[key] = value + } + merged[sbomFormatAnnotation] = format + merged[sbomSha256Annotation] = fingerprint + o.payload.Annotations = merged +} diff --git a/cmd/kosli/attestSbom_test.go b/cmd/kosli/attestSbom_test.go new file mode 100644 index 000000000..b8a765b3d --- /dev/null +++ b/cmd/kosli/attestSbom_test.go @@ -0,0 +1,272 @@ +package main + +import ( + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "runtime" + "syscall" + "testing" + "time" + + "github.com/kosli-dev/cli/internal/requests" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type AttestSbomCommandTestSuite struct { + flowName string + trailName string + suite.Suite + defaultKosliArguments string +} + +func (suite *AttestSbomCommandTestSuite) SetupTest() { + suite.flowName = "attest-sbom" + suite.trailName = "test-123" + global = &GlobalOpts{ + ApiToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ImNkNzg4OTg5In0.e8i_lA_QrEhFncb05Xw6E_tkCHU9QfcY4OLTVUCHffY", + Org: "docs-cmd-test-user", + Host: "http://localhost:8001", + } + suite.defaultKosliArguments = fmt.Sprintf( + " --flow %s --trail %s --repo-root ../.. --host %s --org %s --api-token %s", + suite.flowName, suite.trailName, global.Host, global.Org, global.ApiToken, + ) + CreateFlowWithTemplate(suite.flowName, "testdata/valid_template.yml", suite.T()) + BeginTrail(suite.trailName, suite.flowName, "", suite.T()) +} + +// sizedSbom makes a file of the given size without allocating it: past the +// first byte it is zeros the filesystem never stored. The leading "{" sends the +// parser down the JSON branch, which fails on the second byte, rather than the +// fallback that regex-scans the whole buffer three times. +func (suite *AttestSbomCommandTestSuite) sizedSbom(name string, size int64) string { + path := filepath.Join(suite.T().TempDir(), name) + if err := os.WriteFile(path, []byte("{"), 0644); err != nil { + suite.T().Fatal(err) + } + if err := os.Truncate(path, size); err != nil { + suite.T().Fatal(err) + } + return path +} + +// The dry-run cases are what prove the payload this command builds. They need a +// server for the flow and trail in SetupTest, but not one that knows the sbom +// type, so they stay green while the server side is unreleased. +func (suite *AttestSbomCommandTestSuite) TestAttestSbomBuildsTheRightRequest() { + runTestCmd(suite.T(), []cmdTestCase{ + { + name: "posts to the system endpoint with type_name sbom and the CycloneDX summary", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --dry-run %s", suite.defaultKosliArguments), + goldenRegex: `(?s)trail/test-123/system.*"type_name": "sbom".*"format": "cyclonedx-1\.6".*"original_fingerprint": "db09ef115d88e48a5ef553b21a88ccdc15b3df700e0b7c3736e2ef1024d26d9c".*"package_count": 1`, + }, + { + name: "records the format and the file checksum as annotations", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --dry-run %s", suite.defaultKosliArguments), + goldenRegex: `(?s)"sbom_format": "cyclonedx-1\.6".*"sbom_sha256": "db09ef115d88e48a5ef553b21a88ccdc15b3df700e0b7c3736e2ef1024d26d9c"`, + }, + { + name: "keeps the caller's own annotations alongside the two it derives", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --annotate team=platform --dry-run %s", suite.defaultKosliArguments), + goldenRegex: `"team": "platform"`, + }, + { + name: "still derives its own annotations when the caller supplies one", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --annotate team=platform --dry-run %s", suite.defaultKosliArguments), + goldenRegex: `"sbom_format": "cyclonedx-1\.6"`, + }, + { + name: "reads SPDX as well, and reports the version the file declares", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/spdx.json --dry-run %s", suite.defaultKosliArguments), + goldenRegex: `(?s)"type_name": "sbom".*"format": "spdx-2\.3"`, + }, + }) +} + +func (suite *AttestSbomCommandTestSuite) TestAttestSbomRejectsBadInput() { + tests := []cmdTestCase{ + { + wantError: true, + name: "fails when --sbom-file is missing", + cmd: fmt.Sprintf("attest sbom --name my-sbom %s", suite.defaultKosliArguments), + golden: "Error: required flag(s) \"sbom-file\" not set\n", + }, + { + wantError: true, + name: "fails when --attachments is used too, because two attachments would be compressed", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --attachments testdata/sbom/spdx.json %s", suite.defaultKosliArguments), + golden: "Error: --attachments cannot be used with attest sbom: the SBOM file is the only attachment, and a second one would be compressed\n", + }, + { + // The command wraps the parser's message, and only this exercises + // that wrapping. + wantError: true, + name: "fails when the file is gzipped, because the format is read from it", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/compressed.json.gz %s", suite.defaultKosliArguments), + golden: "Error: failed to parse SBOM file [testdata/sbom/compressed.json.gz]: the file is gzip compressed; supply the uncompressed SBOM\n", + }, + { + wantError: true, + name: "fails when the file is not an SBOM, naming the file", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/not-an-sbom.json %s", suite.defaultKosliArguments), + golden: "Error: failed to parse SBOM file [testdata/sbom/not-an-sbom.json]: not a CycloneDX SBOM: bomFormat is \"\", expected \"CycloneDX\"\n", + }, + { + wantError: true, + name: "fails when a reserved annotation key is supplied", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --annotate sbom_format=mine %s", suite.defaultKosliArguments), + golden: "Error: annotation key 'sbom_format' is set by this command from the SBOM file and cannot be provided with --annotate\n", + }, + { + wantError: true, + name: "fails when the checksum annotation key is supplied, not only the format one", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --annotate sbom_sha256=mine %s", suite.defaultKosliArguments), + golden: "Error: annotation key 'sbom_sha256' is set by this command from the SBOM file and cannot be provided with --annotate\n", + }, + { + wantError: true, + name: "fails when the path is a directory rather than a file", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom %s", suite.defaultKosliArguments), + golden: "Error: SBOM file [testdata/sbom] is a directory; supply the SBOM file itself\n", + }, + } + runTestCmd(suite.T(), tests) +} + +// A shared CI env block is how several attest steps get configured at once, so +// KOSLI_ATTACHMENTS reaches this command without anyone typing the flag. The +// flag is hidden from help, so the error has to say where the value came from, +// and only when that is where it came from. +func (suite *AttestSbomCommandTestSuite) TestAttestSbomRejectsAttachmentsFromTheEnvironment() { + suite.T().Setenv("KOSLI_ATTACHMENTS", "testdata/sbom/spdx.json") + runTestCmd(suite.T(), []cmdTestCase{ + { + wantError: true, + name: "names the environment variable the user did not type", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json %s", suite.defaultKosliArguments), + golden: "Error: --attachments cannot be used with attest sbom (set by environment variable KOSLI_ATTACHMENTS): the SBOM file is the only attachment, and a second one would be compressed\n", + }, + { + // The typed flag wins and the variable is never applied, so blaming + // it would send the user to edit something that had no effect. + wantError: true, + name: "does not blame the environment when the flag was typed as well", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --attachments testdata/sbom/spdx.json %s", suite.defaultKosliArguments), + golden: "Error: --attachments cannot be used with attest sbom: the SBOM file is the only attachment, and a second one would be compressed\n", + }, + }) +} + +func (suite *AttestSbomCommandTestSuite) TestAttestSbomRejectsAttachmentsFromAConfigFile() { + configFile := filepath.Join(suite.T().TempDir(), "kosli.yml") + if err := os.WriteFile(configFile, []byte("attachments: testdata/sbom/spdx.json\n"), 0644); err != nil { + suite.T().Fatal(err) + } + runTestCmd(suite.T(), []cmdTestCase{ + { + wantError: true, + name: "names the config file the value came from", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json --config-file %s %s", configFile, suite.defaultKosliArguments), + golden: fmt.Sprintf("Error: --attachments cannot be used with attest sbom (set by config file [%s]): the SBOM file is the only attachment, and a second one would be compressed\n", configFile), + }, + }) +} + +func (suite *AttestSbomCommandTestSuite) TestAttestSbomSizeLimit() { + over := suite.sizedSbom("oversize.json", maxSbomFileBytes+1) + atLimit := suite.sizedSbom("at-limit.json", maxSbomFileBytes) + + runTestCmd(suite.T(), []cmdTestCase{ + { + wantError: true, + name: "fails locally on an oversize file, rather than with a bare 413 from the server", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file %s %s", over, suite.defaultKosliArguments), + golden: fmt.Sprintf("Error: SBOM file [%s] is above the %d byte limit for an SBOM attestation\n", over, maxSbomFileBytes), + }, + { + // Reaching the parser is the point: it proves the limit is the + // largest accepted size and not the smallest rejected one. + wantError: true, + name: "a file of exactly the limit gets past the size guard", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file %s %s", atLimit, suite.defaultKosliArguments), + goldenRegex: `failed to parse SBOM file .* not valid JSON`, + }, + }) +} + +// The only test that exercises the command against a real server. It fails +// with "System attestation type 'sbom' does not exist" until the server that +// CI tests against carries the type; that red is accurate and is not to be +// skipped around. +func (suite *AttestSbomCommandTestSuite) TestAttestSbomRoundTrip() { + runTestCmd(suite.T(), []cmdTestCase{ + { + name: "reports a CycloneDX SBOM against a trail", + cmd: fmt.Sprintf("attest sbom --name my-sbom --sbom-file testdata/sbom/cyclonedx.json %s", suite.defaultKosliArguments), + golden: "sbom attestation 'my-sbom' is reported to trail: test-123\n", + }, + }) +} + +// The recorded checksum is only verifiable by hand if the bytes the server +// receives are the bytes that were hashed. The uploader is handed those bytes, +// not a path it would read again, so the same buffer serves the fingerprint, +// the summary and the body. Outside the suite because it asserts local logic +// and should still report when no server is running. +func TestSbomUploadsTheBytesItHashed(t *testing.T) { + o := &attestSbomOptions{ + CommonAttestationOptions: &CommonAttestationOptions{fingerprintOptions: &fingerprintOptions{}}, + sbomFilePath: "testdata/sbom/cyclonedx.json", + payload: SbomAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}, TypeName: "sbom"}, + } + 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) + require.Equal(t, "cyclonedx.json", fb.Name) + + uploaded := fmt.Sprintf("%x", sha256.Sum256(fb.Data)) + require.Equal(t, o.payload.AttestationData.OriginalFingerprint, uploaded, "original_fingerprint must describe the uploaded bytes") + require.Equal(t, o.payload.Annotations[sbomSha256Annotation], uploaded, "sbom_sha256 must describe the uploaded bytes") + // Anchor: the digest is of real content, not of an empty buffer. + require.Equal(t, "db09ef115d88e48a5ef553b21a88ccdc15b3df700e0b7c3736e2ef1024d26d9c", uploaded) +} + +// The only guard in loadSbom without a test, and it could not have one while the +// check ran on an already-open handle: open(2) on a fifo with no writer blocks, +// so a reversed order hangs rather than failing. The timeout turns that hang +// into a named failure, which makes the ordering itself something a red run can +// report, not just the branch. +func TestSbomRejectsAFifoWithoutBlocking(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no mkfifo on windows") + } + path := filepath.Join(t.TempDir(), "bom.json") + require.NoError(t, syscall.Mkfifo(path, 0644)) + + o := &attestSbomOptions{ + CommonAttestationOptions: &CommonAttestationOptions{fingerprintOptions: &fingerprintOptions{}}, + sbomFilePath: path, + payload: SbomAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}}, + } + done := make(chan error, 1) + go func() { _, err := o.loadSbom(); done <- err }() + + select { + case err := <-done: + require.ErrorContains(t, err, "is not a regular file") + case <-time.After(5 * time.Second): + t.Fatal("loadSbom blocked on a fifo: the stat must precede the open") + } +} + +func TestAttestSbomCommandTestSuite(t *testing.T) { + suite.Run(t, new(AttestSbomCommandTestSuite)) +} diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 9eda56102..7244801c9 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -292,6 +292,7 @@ Paths the list already matches stay excluded whatever is later added there, so k newComplianceStatusFlag = "The new compliance status to set on the attestation." originalAttestationTypeFlag = "The original attestation type being overridden (e.g. generic, snyk, junit, sonar, jira, pull_request, custom)." attestationDecisionControlFlag = "The control identifier being evaluated (e.g. RCTL-043)." + attestationSbomFileFlag = "The path to the SBOM file. CycloneDX (JSON, XML) and SPDX (JSON, tag-value) are supported." excludeScalingFlag = "[optional] Exclude scaling events for snapshots. Snapshots with scaling changes will not result in new environment records." includeScalingFlag = "[optional] Include scaling events for snapshots. Snapshots with scaling changes will result in new environment records." includedEnvironments = "[optional] Comma separated list of environments to include in logical environment" @@ -773,7 +774,20 @@ func configValueSource(flagName string) string { // Bind each cobra flag to its associated viper configuration // (coming either from environment variables or config file) +// viperAppliedFlags records which flags of the command being run were filled in +// from the environment or the config file rather than typed. Cobra marks both +// as Changed, so this is the only way a command can tell them apart when it +// wants to say where a value came from. Reset on every bindFlags call. +var viperAppliedFlags = map[string]bool{} + +// flagCameFromConfig reports whether the named flag's value was applied from the +// environment or the config file rather than typed on the command line. +func flagCameFromConfig(flagName string) bool { + return viperAppliedFlags[flagName] +} + func bindFlags(cmd *cobra.Command, v *viper.Viper) error { + viperAppliedFlags = map[string]bool{} // A value that cannot be applied to its flag is user input, so it is // returned as an error rather than reported from inside the VisitAll // closure. Reporting it here would reach nobody: the logger's error stream @@ -837,6 +851,7 @@ func bindFlags(cmd *cobra.Command, v *viper.Viper) error { bindErr = errors.Join(bindErr, fmt.Errorf("failed to set flag '--%s' from %s: %v", f.Name, configValueSource(f.Name), err)) } } + viperAppliedFlags[f.Name] = true } }) diff --git a/cmd/kosli/testdata/empty-flag-audit-coverage.json b/cmd/kosli/testdata/empty-flag-audit-coverage.json index 84b07a7b5..58ec70e8e 100644 --- a/cmd/kosli/testdata/empty-flag-audit-coverage.json +++ b/cmd/kosli/testdata/empty-flag-audit-coverage.json @@ -406,6 +406,33 @@ "trail": "string", "user-data": "string" }, + "attest sbom": { + "annotate": "stringToString", + "artifact-type": "string", + "attachments": "stringSlice", + "commit": "string", + "description": "string", + "dry-run": "bool", + "exclude": "stringSlice", + "external-fingerprint": "stringToString", + "external-url": "stringToString", + "fingerprint": "string", + "flow": "string", + "name": "string", + "origin-url": "string", + "redact-commit-info": "stringSlice", + "registry-password": "string", + "registry-provider": "string", + "registry-username": "string", + "repo-id": "string", + "repo-provider": "string", + "repo-root": "string", + "repo-url": "string", + "repository": "string", + "sbom-file": "string", + "trail": "string", + "user-data": "string" + }, "attest snyk": { "annotate": "stringToString", "artifact-type": "string", diff --git a/cmd/kosli/testdata/sbom/compressed.json.gz b/cmd/kosli/testdata/sbom/compressed.json.gz new file mode 100644 index 000000000..1cab47bf1 Binary files /dev/null and b/cmd/kosli/testdata/sbom/compressed.json.gz differ diff --git a/cmd/kosli/testdata/sbom/cyclonedx.json b/cmd/kosli/testdata/sbom/cyclonedx.json new file mode 100644 index 000000000..641cdbaac --- /dev/null +++ b/cmd/kosli/testdata/sbom/cyclonedx.json @@ -0,0 +1,14 @@ +{ + "bomFormat": "CycloneDX", + "specVersion": "1.6", + "version": 1, + "metadata": { + "timestamp": "2026-09-11T10:00:00Z", + "tools": { "components": [{ "type": "application", "name": "syft", "version": "1.50.0" }] }, + "component": { "type": "application", "name": "app", "version": "1.2.3" } + }, + "components": [ + { "type": "library", "name": "openssl", "version": "3.0.2" }, + { "type": "file", "name": "/etc/hosts" } + ] +} diff --git a/cmd/kosli/testdata/sbom/not-an-sbom.json b/cmd/kosli/testdata/sbom/not-an-sbom.json new file mode 100644 index 000000000..38ff1c8e5 --- /dev/null +++ b/cmd/kosli/testdata/sbom/not-an-sbom.json @@ -0,0 +1 @@ +{"hello":"world"} diff --git a/cmd/kosli/testdata/sbom/spdx.json b/cmd/kosli/testdata/sbom/spdx.json new file mode 100644 index 000000000..7cbb81fc1 --- /dev/null +++ b/cmd/kosli/testdata/sbom/spdx.json @@ -0,0 +1,16 @@ +{ + "spdxVersion": "SPDX-2.3", + "dataLicense": "CC0-1.0", + "SPDXID": "SPDXRef-DOCUMENT", + "name": "minimal", + "documentNamespace": "https://example.com/minimal", + "creationInfo": { "created": "2026-09-11T10:00:00Z", "creators": ["Tool: syft-1.50.0"] }, + "packages": [ + { "SPDXID": "SPDXRef-Package-app", "name": "app", "versionInfo": "1.2.3", + "downloadLocation": "NOASSERTION", "filesAnalyzed": false } + ], + "relationships": [ + { "spdxElementId": "SPDXRef-DOCUMENT", "relatedSpdxElement": "SPDXRef-Package-app", + "relationshipType": "DESCRIBES" } + ] +} diff --git a/hack/empty-flag-audit/bootstrap.py b/hack/empty-flag-audit/bootstrap.py index a3eac760a..8f1e16051 100755 --- a/hack/empty-flag-audit/bootstrap.py +++ b/hack/empty-flag-audit/bootstrap.py @@ -154,6 +154,7 @@ "paths-file": "hack/empty-flag-audit/paths.yml", "template-file": "cmd/kosli/testdata/valid_template.yml", "scan-results": "cmd/kosli/testdata/snyk_scan_example.json", + "sbom-file": "cmd/kosli/testdata/sbom/cyclonedx.json", "user-data": ARTIFACT_PATH, "input-file": ARTIFACT_PATH, "attestation-data": ARTIFACT_PATH, @@ -567,7 +568,9 @@ def main(): state = "ok " if entry["baseline_ok"] else ("needs" if reason else "FAIL") print(f"{state} {command}" + ("" if entry["baseline_ok"] else f" {entry.get('error','')[:90]}")) - SPEC.write_text(json.dumps(spec, indent=1)) + # indent=2 matches the committed file. At indent=1 every regeneration + # reindents all 4680 lines and buries the one entry that changed. + SPEC.write_text(json.dumps(spec, indent=2) + "\n") print(f"\nwrote {SPEC}") diff --git a/hack/empty-flag-audit/results.tsv b/hack/empty-flag-audit/results.tsv index e17e38f52..233baddc0 100644 --- a/hack/empty-flag-audit/results.tsv +++ b/hack/empty-flag-audit/results.tsv @@ -358,6 +358,31 @@ attest pullrequest gitlab --repo-url 1 1 1 cli differs differs 0.3 Error: [kosl attest pullrequest gitlab --repository 1 1 1 cli differs differs 0.3 Error: [kosli attest pullrequest gitlab flow=probe-attest-pullrequest-gitlab-repository-fl trail=probe-attest-pullrequest-gitlab-repository-tr] flag '--reposito attest pullrequest gitlab --trail 1 1 1 cli differs differs 0.3 Error: [kosli attest pullrequest gitlab flow=probe-attest-pullrequest-gitlab-trail-fl] flag '--trail' was given an empty value attest pullrequest gitlab --user-data 1 1 1 cli differs differs 0.3 Error: [kosli attest pullrequest gitlab flow=probe-attest-pullrequest-gitlab-user-data-fl trail=probe-attest-pullrequest-gitlab-user-data-tr] flag '--user-data' +attest sbom --annotate 1 0 0 cli differs differs 0.5 Error: [kosli attest sbom flow=probe-attest-sbom-annotate-fl trail=probe-attest-sbom-annotate-tr] flag '--annotate' was given an empty value +attest sbom --artifact-type 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-artifact-type-fl trail=probe-attest-sbom-artifact-type-tr] flag '--artifact-type' was given an empty value +attest sbom --attachments 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-attachments-fl trail=probe-attest-sbom-attachments-tr] flag '--attachments' was given an empty value +attest sbom --commit 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-commit-fl trail=probe-attest-sbom-commit-tr] flag '--commit' was given an empty value +attest sbom --description 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-description-fl trail=probe-attest-sbom-description-tr] flag '--description' was given an empty value +attest sbom --dry-run 1 0 0 cli differs differs 0.4 Error: flag '--dry-run' was given an empty value +attest sbom --exclude 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-exclude-fl trail=probe-attest-sbom-exclude-tr] flag '--exclude' was given an empty value +attest sbom --external-fingerprint 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-external-fingerprint-fl trail=probe-attest-sbom-external-fingerprint-tr] flag '--external-fingerprint' was give +attest sbom --external-url 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-external-url-fl trail=probe-attest-sbom-external-url-tr] flag '--external-url' was given an empty value +attest sbom --fingerprint 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-fingerprint-fl trail=probe-attest-sbom-fingerprint-tr] flag '--fingerprint' was given an empty value +attest sbom --flow 1 1 0 cli differs differs 0.4 Error: [kosli attest sbom trail=probe-attest-sbom-flow-tr] flag '--flow' was given an empty value +attest sbom --name 1 1 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-name-fl trail=probe-attest-sbom-name-tr] flag '--name' was given an empty value +attest sbom --origin-url 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-origin-url-fl trail=probe-attest-sbom-origin-url-tr] flag '--origin-url' was given an empty value +attest sbom --redact-commit-info 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-redact-commit-info-fl trail=probe-attest-sbom-redact-commit-info-tr] flag '--redact-commit-info' was given an e +attest sbom --registry-password 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-registry-password-fl trail=probe-attest-sbom-registry-password-tr] flag '--registry-password' was given an empt +attest sbom --registry-provider 1 0 0 cli differs differs 0.5 Error: [kosli attest sbom flow=probe-attest-sbom-registry-provider-fl trail=probe-attest-sbom-registry-provider-tr] flag '--registry-provider' was given an empt +attest sbom --registry-username 1 0 1 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-registry-username-fl trail=probe-attest-sbom-registry-username-tr] flag '--registry-username' was given an empt +attest sbom --repo-id 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-repo-id-fl trail=probe-attest-sbom-repo-id-tr] flag '--repo-id' was given an empty value +attest sbom --repo-provider 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-repo-provider-fl trail=probe-attest-sbom-repo-provider-tr] flag '--repo-provider' was given an empty value +attest sbom --repo-root 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-repo-root-fl trail=probe-attest-sbom-repo-root-tr] flag '--repo-root' was given an empty value +attest sbom --repo-url 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-repo-url-fl trail=probe-attest-sbom-repo-url-tr] flag '--repo-url' was given an empty value +attest sbom --repository 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-repository-fl trail=probe-attest-sbom-repository-tr] flag '--repository' was given an empty value +attest sbom --sbom-file 1 1 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-sbom-file-fl trail=probe-attest-sbom-sbom-file-tr] flag '--sbom-file' was given an empty value +attest sbom --trail 1 1 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-trail-fl] flag '--trail' was given an empty value +attest sbom --user-data 1 0 0 cli differs differs 0.4 Error: [kosli attest sbom flow=probe-attest-sbom-user-data-fl trail=probe-attest-sbom-user-data-tr] flag '--user-data' was given an empty value attest snyk --annotate 1 1 1 cli differs differs 0.3 Error: [kosli attest snyk flow=probe-attest-snyk-annotate-fl trail=probe-attest-snyk-annotate-tr] flag '--annotate' was given an empty value attest snyk --artifact-type 1 1 1 cli differs differs 0.3 Error: [kosli attest snyk flow=probe-attest-snyk-artifact-type-fl trail=probe-attest-snyk-artifact-type-tr] flag '--artifact-type' was given an empty value attest snyk --attachments 1 1 1 cli differs differs 0.4 Error: [kosli attest snyk flow=probe-attest-snyk-attachments-fl trail=probe-attest-snyk-attachments-tr] flag '--attachments' was given an empty value diff --git a/hack/empty-flag-audit/spec.json b/hack/empty-flag-audit/spec.json index 10b844ad4..c865d4903 100644 --- a/hack/empty-flag-audit/spec.json +++ b/hack/empty-flag-audit/spec.json @@ -4676,5 +4676,101 @@ "flag_values": {}, "setup": [], "verify": [] + }, + "attest sbom": { + "args": [], + "flags": { + "flow": "{flow}", + "name": "{name}", + "sbom-file": "cmd/kosli/testdata/sbom/cyclonedx.json", + "trail": "{trail}" + }, + "baseline_ok": true, + "baseline_exit": 0, + "baseline_output": "[warning] Repo information will not be reported as ID, Name and URL are required.\nsbom attestation '{name}' is reported to trail: {trail}", + "flags_to_test": [ + "annotate", + "artifact-type", + "attachments", + "commit", + "description", + "dry-run", + "exclude", + "external-fingerprint", + "external-url", + "fingerprint", + "flow", + "name", + "origin-url", + "redact-commit-info", + "registry-password", + "registry-provider", + "registry-username", + "repo-id", + "repo-provider", + "repo-root", + "repo-url", + "repository", + "sbom-file", + "trail", + "user-data" + ], + "flag_values": { + "annotate": "probe=annotate", + "artifact-type": "file", + "attachments": "cmd/kosli/testdata/person-schema.json", + "commit": "HEAD", + "description": "probe-description", + "dry-run": "true", + "exclude": "probe-exclude", + "external-fingerprint": "probe=1bef738d0bb1e690500f99a5b57d958caf3a5eb3e00d9012e1f4369fc6812e01", + "external-url": "probe=http://example.com", + "fingerprint": "1bef738d0bb1e690500f99a5b57d958caf3a5eb3e00d9012e1f4369fc6812e01", + "flow": "{flow}", + "name": "{name}", + "origin-url": "http://example.com", + "redact-commit-info": "author", + "registry-password": "probe-registry-password", + "registry-provider": "probe-registry-provider", + "registry-username": "probe-registry-username", + "repo-id": "probe-repo-id", + "repo-provider": "github", + "repo-root": ".", + "repo-url": "http://example.com", + "repository": "probe-repository", + "sbom-file": "cmd/kosli/testdata/sbom/cyclonedx.json", + "trail": "{trail}", + "user-data": "cmd/kosli/testdata/person-schema.json" + }, + "setup": [ + { + "argv": [ + "create", + "flow", + "{flow}", + "--use-empty-template" + ] + }, + { + "argv": [ + "begin", + "trail", + "{trail}", + "--flow", + "{flow}" + ] + } + ], + "verify": [ + [ + "get", + "trail", + "{trail}", + "--flow", + "{flow}", + "--output", + "json" + ] + ] } } diff --git a/internal/requests/requests.go b/internal/requests/requests.go index 5c1cba76a..920d427ec 100644 --- a/internal/requests/requests.go +++ b/internal/requests/requests.go @@ -26,6 +26,15 @@ type FormItem struct { Content interface{} } +// FileBytes is the Content of a "file-bytes" FormItem: a file the caller has +// already read. Uploading these bytes, rather than a path the body builder +// would open again, is what lets a caller promise that a checksum it recorded +// describes the bytes the server received. +type FileBytes struct { + Name string + Data []byte +} + // HTTPResponse is a wrapper of http.Response with ready-extracted string body type HTTPResponse struct { Body string @@ -215,8 +224,10 @@ func createMultipartRequestBody(items []FormItem) (string, *bytes.Buffer, map[st jsonFields[item.FieldName] = jsonBytes case "file": - // Handle file upload separately - filename := item.Content.(string) + filename, ok := item.Content.(string) + if !ok { + return "", body, nil, fmt.Errorf("form item %s: file content must be a path", item.FieldName) + } file, err := os.Open(filename) if err != nil { return "", body, nil, err @@ -237,6 +248,25 @@ func createMultipartRequestBody(items []FormItem) (string, *bytes.Buffer, map[st if err != nil { return "", body, nil, err } + + case "file-bytes": + fb, ok := item.Content.(FileBytes) + if !ok { + return "", body, nil, fmt.Errorf("form item %s: file-bytes content must be a FileBytes", item.FieldName) + } + part, err := writer.CreateFormFile(item.FieldName, fb.Name) + if err != nil { + return "", body, nil, err + } + _, err = part.Write(fb.Data) + if err != nil { + return "", body, nil, err + } + + default: + // Skipping silently would post a body with a part missing, and a + // caller that recorded a checksum for that part would never know. + return "", body, nil, fmt.Errorf("form item %s: unknown type %q", item.FieldName, item.Type) } } contentType := writer.FormDataContentType() diff --git a/internal/requests/requests_test.go b/internal/requests/requests_test.go index 241793274..ce969632a 100644 --- a/internal/requests/requests_test.go +++ b/internal/requests/requests_test.go @@ -637,3 +637,30 @@ func (suite *RequestsTestSuite) TestNonMultipartJSON_IsCompact() { func TestRequestsTestSuite(t *testing.T) { suite.Run(t, new(RequestsTestSuite)) } + +// A file-bytes item must put exactly the caller's bytes on the wire, under the +// caller's name. This is what lets an attestation promise that a checksum it +// recorded describes what the server received. +func TestCreateMultipartRequestBodyFileBytes(t *testing.T) { + data := []byte(`{"bomFormat":"CycloneDX"}`) + _, body, jsonFields, err := createMultipartRequestBody([]FormItem{ + {Type: "field", FieldName: "data_json", Content: map[string]string{"a": "b"}}, + {Type: "file-bytes", FieldName: "attachment_file", Content: FileBytes{Name: "bom.json", Data: data}}, + }) + require.NoError(t, err) + raw := body.String() + require.Contains(t, raw, `name="attachment_file"; filename="bom.json"`) + require.Contains(t, raw, string(data)) + // Only JSON fields are surfaced for dry-run logging; the file is not one of them. + require.NotContains(t, jsonFields, "attachment_file") + + _, _, _, err = createMultipartRequestBody([]FormItem{ + {Type: "file-bytes", FieldName: "attachment_file", Content: "a/path/instead"}, + }) + require.Error(t, err, "a path must not be accepted where bytes are expected") + + _, _, _, err = createMultipartRequestBody([]FormItem{ + {Type: "file_bytes", FieldName: "attachment_file", Content: FileBytes{Name: "bom.json", Data: data}}, + }) + require.Error(t, err, "a misspelled type must fail, not silently drop the part") +} diff --git a/internal/sbom/sbom.go b/internal/sbom/sbom.go index 10fbd7df6..743496591 100644 --- a/internal/sbom/sbom.go +++ b/internal/sbom/sbom.go @@ -70,15 +70,21 @@ var ( // ProcessSBOMFile reads an SBOM file and returns its format and a normalised // summary. It confirms the file identifies itself as the format it parses as; // it does not validate against the format's schema. +// +// It reads the whole file with no size limit. A caller that needs one, as the +// attest command does, reads the bytes itself and calls ProcessSBOM. func ProcessSBOMFile(file string) (*SBOMData, error) { content, err := os.ReadFile(file) if err != nil { return nil, err } - return processSBOM(content) + return ProcessSBOM(content) } -func processSBOM(content []byte) (*SBOMData, error) { +// ProcessSBOM is ProcessSBOMFile for content already in memory, so a caller +// that must also fingerprint the file can do both from one read rather than +// risk the two describing different bytes. +func ProcessSBOM(content []byte) (*SBOMData, error) { if bytes.HasPrefix(content, gzipMagic) { return nil, fmt.Errorf("the file is gzip compressed; supply the uncompressed SBOM") }