diff --git a/.claude/skills/new-command/references/archetype-attest.md b/.claude/skills/new-command/references/archetype-attest.md index fdd4b5862..7c85b6886 100644 --- a/.claude/skills/new-command/references/archetype-attest.md +++ b/.claude/skills/new-command/references/archetype-attest.md @@ -53,7 +53,9 @@ Canonical example: `cmd/kosli/attestCustom.go` — read it in full and adapt. - `RequireFlags(cmd, []string{"flow", "trail", "name", ...})` for type-specific required flags. **`RunE`** -- Capture `o.repoURLExplicit = cmd.Flags().Changed("repo-url")` before delegating. +- Capture `o.repoURLExplicit = cmd.Flags().Changed("repo-url")` and `o.repoNameExplicit = cmd.Flags().Changed("repository")` before delegating. +- Do not capture `--commit` or `--repo-root`: `addAttestationFlags` keeps the flag set on the options and `CommonAttestationOptions.run` reads `Changed` from it. +- If the command cannot work without the commit (as `attest pullrequest *` and `attest jira` cannot), set `o.commitRequiredFor = ""` in `run` before delegating. **`run` method** - Build the URL: `url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "")`. diff --git a/cmd/kosli/attestJira.go b/cmd/kosli/attestJira.go index 84556acc5..c3849f224 100644 --- a/cmd/kosli/attestJira.go +++ b/cmd/kosli/attestJira.go @@ -330,6 +330,7 @@ func (o *attestJiraOptions) run(args []string) error { return err } + o.commitRequiredFor = "search for Jira issue keys" err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index e0d747156..bc1605214 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -9,6 +9,7 @@ import ( "github.com/kosli-dev/cli/internal/gitview" "github.com/kosli-dev/cli/internal/requests" + "github.com/spf13/pflag" ) const commitDescription = `You can optionally associate the attestation to a git commit using ^--commit^ (requires access to a git repo). @@ -56,6 +57,11 @@ type CommonAttestationOptions struct { repoProvider string repoURLExplicit bool repoNameExplicit bool + // flags lets run tell a passed flag from a defaulted one. + flags *pflag.FlagSet + // commitRequiredFor completes "the commit is required to ..."; empty when + // commit info is optional. + commitRequiredFor string } func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error { @@ -80,15 +86,18 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation } if o.commitSHA != "" { - gv, err := gitview.New(o.srcRepoRoot) + payload.Commit, err = commitInfoRequest{ + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + flags: o.flags, + requiredFor: o.commitRequiredFor, + }.resolve() if err != nil { - return fmt.Errorf("failed to get commit info. %s", err) + return err } - commitInfo, err := gv.GetCommitInfoFromCommitSHA(o.commitSHA, false, o.redactedCommitInfo) - if err != nil { - return fmt.Errorf("failed to get commit info. %s", err) - } - payload.Commit = &commitInfo.BasicCommitInfo + } else if o.commitRequiredFor != "" { + return fmt.Errorf("no commit info is available, and the commit is required to %s. Pass --commit", o.commitRequiredFor) } payload.GitRepoInfo, err = getGitRepoInfoFromEnvironment() @@ -117,6 +126,61 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation return err } +type commitInfoRequest struct { + repoRoot string + sha string + redacted []string + flags *pflag.FlagSet + requiredFor string +} + +// lookup reads the commit info from the repository at repoRoot, or returns +// the error from opening it or resolving sha within it. +func (r commitInfoRequest) lookup() (*gitview.CommitInfo, error) { + gv, err := gitview.New(r.repoRoot) + if err != nil { + return nil, err + } + return gv.GetCommitInfoFromCommitSHA(r.sha, false, r.redacted) +} + +func (r commitInfoRequest) commitExplicit() bool { + return r.flags != nil && r.flags.Changed("commit") +} + +// repoRootExplicit is true only when --repo-root carries a value other than +// its "." default: bindFlags marks a config or env value as Changed even when +// it equals the default, and "." itself asks for nothing. +func (r commitInfoRequest) repoRootExplicit() bool { + return r.flags != nil && r.flags.Changed("repo-root") && r.repoRoot != "." +} + +// resolve returns nil, nil when the lookup fails but nothing was asked for +// explicitly and the command can do without the commit: a CI-defaulted +// --commit must not fail a job with no checked-out repository +// (kosli-dev/server#6094). An unresolvable commit, as in a shallow clone, +// deliberately takes the same route. +func (r commitInfoRequest) resolve() (*gitview.BasicCommitInfo, error) { + commitInfo, err := r.lookup() + if err == nil { + return &commitInfo.BasicCommitInfo, nil + } + + describedCommit := "--commit " + r.sha + if !r.commitExplicit() { + describedCommit += " (defaulted from the CI environment)" + } + switch { + case r.requiredFor != "": + return nil, fmt.Errorf("failed to get commit info for %s: %s. The commit is required to %s, so point --repo-root at a repository containing it", describedCommit, err, r.requiredFor) + case r.commitExplicit() || r.repoRootExplicit(): + return nil, fmt.Errorf("failed to get commit info for %s: %s. Point --repo-root at a repository containing it", describedCommit, err) + } + logger.Warn("proceeding without commit info: %s could not be read: %s.", describedCommit, err) + logger.Warn("Kosli binds an attestation reported before its artifact through this commit, so point --repo-root at a repository containing it if that binding is needed.") + return nil, nil +} + // mergeGitRepoInfo applies flag overrides onto base (which may be nil) and // returns nil if ID, Name, or URL is still empty after merging, so that the // field is omitted from the JSON payload. diff --git a/cmd/kosli/attestationCommitInfo_test.go b/cmd/kosli/attestationCommitInfo_test.go new file mode 100644 index 000000000..018773dab --- /dev/null +++ b/cmd/kosli/attestationCommitInfo_test.go @@ -0,0 +1,191 @@ +package main + +import ( + "os" + "testing" + + "github.com/go-git/go-git/v5" + "github.com/stretchr/testify/suite" +) + +// AttestationCommitInfoTestSuite covers how a failed commit lookup is reported +// (kosli-dev/server#6094, kosli-dev/server#5615). The CI default exists only +// while KOSLI_TESTS is unset, because DefaultValue returns "" under it, so inCI +// unsets it around the command run. +type AttestationCommitInfoTestSuite struct { + suite.Suite + headHash string + defaultKosliArguments string +} + +const ( + commitInfoTestFingerprint = "7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9" + // A well-formed SHA that is not in this repository, as in a shallow clone. + absentSHA = "0d4c1e1b7f5c2a9e8b3d6f0a1c4e7b2d5a8f3c60" +) + +func (suite *AttestationCommitInfoTestSuite) SetupTest() { + repo, err := git.PlainOpen("../..") + suite.Require().NoError(err) + head, err := repo.Head() + suite.Require().NoError(err) + suite.headHash = head.Hash().String() + + global = &GlobalOpts{ + ApiToken: "DRY_RUN", + Org: "test-org", + Host: "http://localhost:8001", + DryRun: true, + } + suite.defaultKosliArguments = " --dry-run --host http://localhost:8001 --org test-org --api-token DRY_RUN" +} + +// inCI runs f with the CI defaults live, as in a GitHub Actions job whose +// GITHUB_SHA is sha. +func (suite *AttestationCommitInfoTestSuite) inCI(sha string, f func()) { + if value, set := os.LookupEnv("KOSLI_TESTS"); set { + suite.Require().NoError(os.Unsetenv("KOSLI_TESTS")) + defer func() { suite.Require().NoError(os.Setenv("KOSLI_TESTS", value)) }() + } + suite.T().Setenv("GITHUB_RUN_NUMBER", "1") + suite.T().Setenv("GITHUB_SHA", sha) + f() +} + +func (suite *AttestationCommitInfoTestSuite) attestGeneric(extraFlags string) string { + return "attest generic --fingerprint " + commitInfoTestFingerprint + " --name foo --flow f --trail t " + extraFlags + suite.defaultKosliArguments +} + +func (suite *AttestationCommitInfoTestSuite) beginTrail(extraFlags string) string { + return "begin trail t --flow f " + extraFlags + suite.defaultKosliArguments +} + +func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitWithoutRepositoryWarns() { + // --repo-root defaults to ".", so run where there is no repository at all. + suite.T().Chdir(suite.T().TempDir()) + suite.inCI(suite.headHash, func() { + for _, cmd := range []string{suite.attestGeneric(""), suite.beginTrail("")} { + _, out, _, _, err := executeCommandC(cmd) + suite.Require().NoError(err, cmd) + suite.Contains(out, "[warning] proceeding without commit info", cmd) + suite.Contains(out, "--commit "+suite.headHash+" (defaulted from the CI environment)", cmd) + suite.Contains(out, "repository does not exist", cmd) + suite.Contains(out, "THIS IS A DRY-RUN", cmd) + suite.NotContains(out, "git_commit_info", cmd) + } + }) +} + +func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitNotInRepositoryWarns() { + suite.T().Chdir("../..") + suite.inCI(absentSHA, func() { + _, out, _, _, err := executeCommandC(suite.attestGeneric("")) + suite.Require().NoError(err) + suite.Contains(out, "[warning] proceeding without commit info") + suite.Contains(out, "--commit "+absentSHA+" (defaulted from the CI environment)") + suite.NotContains(out, "git_commit_info") + }) +} + +func (suite *AttestationCommitInfoTestSuite) TestCommitFromEnvVarIsExplicit() { + suite.T().Chdir(suite.T().TempDir()) + suite.T().Setenv("KOSLI_COMMIT", suite.headHash) + _, out, _, _, err := executeCommandC(suite.attestGeneric("")) + suite.Require().Error(err) + suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+":") + suite.NotContains(err.Error(), "defaulted from the CI environment") + suite.NotContains(out, "[warning] proceeding without commit info") +} + +// A --repo-root set via the environment or config to its own "." default +// must not count as explicit: bindFlags marks the flag Changed regardless of +// whether the applied value differs from the default. +func (suite *AttestationCommitInfoTestSuite) TestRepoRootFromEnvVarAtDefaultValueStillWarns() { + suite.T().Chdir(suite.T().TempDir()) + suite.T().Setenv("KOSLI_REPO_ROOT", ".") + suite.inCI(suite.headHash, func() { + _, out, _, _, err := executeCommandC(suite.attestGeneric("")) + suite.Require().NoError(err) + suite.Contains(out, "[warning] proceeding without commit info") + }) +} + +func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitWithExplicitRepoRootFails() { + suite.inCI(suite.headHash, func() { + for _, cmd := range []string{suite.attestGeneric("--repo-root testdata"), suite.beginTrail("--repo-root testdata")} { + _, out, _, _, err := executeCommandC(cmd) + suite.Require().Error(err, cmd) + suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+" (defaulted from the CI environment)", cmd) + suite.Contains(err.Error(), "repository does not exist", cmd) + suite.Contains(err.Error(), "Point --repo-root at a repository containing it", cmd) + suite.NotContains(out, "[warning] proceeding without commit info", cmd) + } + }) +} + +func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitWithoutRepositoryFails() { + tests := []cmdTestCase{ + { + wantError: true, + name: "attest generic: an explicit --commit fails when --repo-root has no repository", + cmd: suite.attestGeneric("--commit " + suite.headHash + " --repo-root testdata"), + goldenRegex: "Error: failed to get commit info for --commit " + suite.headHash + ": .*repository does not exist\\. Point --repo-root at a repository containing it\n", + }, + { + wantError: true, + name: "begin trail: an explicit --commit fails when --repo-root has no repository", + cmd: suite.beginTrail("--commit " + suite.headHash + " --repo-root testdata"), + goldenRegex: "Error: failed to get commit info for --commit " + suite.headHash + ": .*repository does not exist\\. Point --repo-root at a repository containing it\n", + }, + } + runTestCmd(suite.T(), tests) +} + +func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitIsAttached() { + tests := []cmdTestCase{ + { + name: "attest generic: an explicit --commit is resolved and sent", + cmd: suite.attestGeneric("--commit " + suite.headHash + " --repo-root ../.."), + goldenRegex: `(?s)"git_commit_info": \{.*"sha1": "` + suite.headHash + `"`, + }, + { + name: "begin trail: an explicit --commit is resolved and sent", + cmd: suite.beginTrail("--commit " + suite.headHash + " --repo-root ../.."), + goldenRegex: `(?s)"git_commit_info": \{.*"sha1": "` + suite.headHash + `"`, + }, + } + runTestCmd(suite.T(), tests) +} + +func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() { + suite.T().Chdir(suite.T().TempDir()) + suite.inCI(suite.headHash, func() { + for _, tc := range []struct{ cmd, need string }{ + {"attest pullrequest github --name foo --flow f --trail t --github-token tok --github-org o --repository r" + suite.defaultKosliArguments, "find pull requests"}, + {"attest jira --name foo --flow f --trail t --jira-base-url https://x.atlassian.net --jira-username u --jira-api-token tok" + suite.defaultKosliArguments, "search for Jira issue keys"}, + } { + _, out, _, _, err := executeCommandC(tc.cmd) + suite.Require().Error(err, tc.cmd) + suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+" (defaulted from the CI environment)", tc.cmd) + suite.Contains(err.Error(), "The commit is required to "+tc.need, tc.cmd) + suite.NotContains(out, "[warning] proceeding without commit info", tc.cmd) + } + }) +} + +// Unreachable from the CLI while RequireFlags keeps --commit non-empty; guards +// the nil dereference that would follow if that changed. +func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFailWithoutOne() { + o := &CommonAttestationOptions{ + fingerprintOptions: &fingerprintOptions{}, + attestationNameTemplate: "foo", + commitRequiredFor: "find pull requests", + } + err := o.run([]string{}, &CommonAttestationPayload{}) + suite.Require().Error(err) + suite.Contains(err.Error(), "the commit is required to find pull requests") +} + +func TestAttestationCommitInfoTestSuite(t *testing.T) { + suite.Run(t, new(AttestationCommitInfoTestSuite)) +} diff --git a/cmd/kosli/beginTrail.go b/cmd/kosli/beginTrail.go index 8a796b34f..2d973f932 100644 --- a/cmd/kosli/beginTrail.go +++ b/cmd/kosli/beginTrail.go @@ -9,6 +9,7 @@ import ( "github.com/kosli-dev/cli/internal/gitview" "github.com/kosli-dev/cli/internal/requests" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) const beginTrailShortDesc = `Begin or update a Kosli flow trail.` @@ -50,6 +51,9 @@ type beginTrailOptions struct { repoURL string repoProvider string repoNameExplicit bool + // flags lets commitInfoRequest tell a passed --commit/--repo-root from a + // defaulted one. + flags *pflag.FlagSet } type TrailPayload struct { @@ -85,6 +89,7 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { o.repoNameExplicit = cmd.Flags().Changed("repository") + o.flags = cmd.Flags() return o.run(args) }, } @@ -128,15 +133,15 @@ func (o *beginTrailOptions) run(args []string) error { } if o.commitSHA != "" { - gv, err := gitview.New(o.srcRepoRoot) + o.payload.Commit, err = commitInfoRequest{ + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + flags: o.flags, + }.resolve() if err != nil { return err } - commitInfo, err := gv.GetCommitInfoFromCommitSHA(o.commitSHA, false, o.redactedCommitInfo) - if err != nil { - return err - } - o.payload.Commit = &commitInfo.BasicCommitInfo } base, err := getGitRepoInfoFromEnvironment() diff --git a/cmd/kosli/flags.go b/cmd/kosli/flags.go index 4b42f3bd5..7aadb1284 100644 --- a/cmd/kosli/flags.go +++ b/cmd/kosli/flags.go @@ -95,6 +95,7 @@ func addListFlags(cmd *cobra.Command, o *listOptions, customPageLimit ...int) { } func addAttestationFlags(cmd *cobra.Command, o *CommonAttestationOptions, payload *CommonAttestationPayload, ci string) { + o.flags = cmd.Flags() commitFlagDesc := attestationCommitFlag if _, ok := cmd.Annotations["pr"]; ok { commitFlagDesc = "the git merge commit to be checked for associated pull requests." diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index 5f1c329d5..e78da1cf3 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -33,6 +33,7 @@ func (o *attestPROptions) run(args []string) error { return err } + o.commitRequiredFor = "find pull requests" err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload) if err != nil { return err diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 32c689145..2f67671c9 100644 --- a/cmd/kosli/root.go +++ b/cmd/kosli/root.go @@ -282,7 +282,7 @@ Paths the list already matches stay excluded whatever is later added there, so k intervalFlag = "[optional] Expression to define specified snapshots range." showUnchangedArtifactsFlag = "[defaulted] Show the unchanged artifacts present in both snapshots within the diff output." attestationFingerprintFlag = "[conditional] The SHA256 fingerprint of the artifact to attach the attestation to. Only required if the attestation is for an artifact and --artifact-type and artifact name/path are not used." - attestationCommitFlag = "[conditional] The git commit for which the attestation is associated to. Becomes required when reporting an attestation for an artifact before reporting it to Kosli. (defaulted in some CIs: https://docs.kosli.com/integrations/ci_cd )." + attestationCommitFlag = "[conditional] The git commit for which the attestation is associated to. Becomes required when reporting an attestation for an artifact before reporting it to Kosli. (defaulted in some CIs: https://docs.kosli.com/integrations/ci_cd ). If both --commit and --repo-root are left at their defaults and the commit cannot be read from the repository, a warning is printed and the attestation is sent without commit info." attestationRedactCommitInfoFlag = "[optional] The list of commit info to be redacted before sending to Kosli. Allowed values are one or more of [author, message, branch]." attestationOriginUrlFlag = "[optional] The url pointing to where the attestation came from or is related. (defaulted to the CI url in some CIs: https://docs.kosli.com/integrations/ci_cd/#defaulted-kosli-command-flags-from-ci-variables )." attestationNameFlag = "The name of the attestation as declared in the flow or trail yaml template." @@ -293,7 +293,7 @@ Paths the list already matches stay excluded whatever is later added there, so k uploadJunitResultsFlag = "[defaulted] Whether to upload the provided Junit results directory as an attachment to Kosli or not." uploadSnykResultsFlag = "[defaulted] Whether to upload the provided Snyk results file as an attachment to Kosli or not." attestationAssertFlag = "[optional] Exit with non-zero code if the attestation is non-compliant" - beginTrailCommitFlag = "[defaulted] The git commit from which the trail is begun. (defaulted in some CIs: https://docs.kosli.com/integrations/ci_cd, otherwise defaults to HEAD )." + beginTrailCommitFlag = "[defaulted] The git commit from which the trail is begun. (defaulted in some CIs: https://docs.kosli.com/integrations/ci_cd, otherwise unset ). If both --commit and --repo-root are left at their defaults and the commit cannot be read from the repository, a warning is printed and the trail is begun without commit info." attachmentsFlag = "[optional] The comma-separated list of paths of attachments for the reported attestation. Attachments can be files or directories. All attachments are compressed and uploaded to Kosli's evidence vault." externalFingerprintFlag = "[optional] A SHA256 fingerprint of an external attachment represented by --external-url. The format is label=fingerprint (labels cannot contain '.' or '='). This flag can be set multiple times. There must be an external url with a matching label for each external fingerprint." externalURLFlag = "[optional] Add labeled reference URL for an external resource. The format is label=url (labels cannot contain '.' or '='). This flag can be set multiple times. If the resource is a file or dir, you can optionally add its fingerprint via --external-fingerprint" diff --git a/cmd/kosli/testdata/output/docs/mintlify/snyk.md b/cmd/kosli/testdata/output/docs/mintlify/snyk.md index 73c4e83b0..9811045b3 100644 --- a/cmd/kosli/testdata/output/docs/mintlify/snyk.md +++ b/cmd/kosli/testdata/output/docs/mintlify/snyk.md @@ -45,7 +45,7 @@ In other CI systems, set them explicitly to capture repository metadata. | `--annotate` | stringToString | [optional] Annotate the attestation with data using key=value. | | `-t`, `--artifact-type` | string | The type of the artifact to calculate its SHA256 fingerprint. One of: [oci, docker, file, dir]. Only required if you want Kosli to calculate the fingerprint for you (i.e. when you don't specify '`--fingerprint`' on commands that allow it). | | `--attachments` | strings | [optional] The comma-separated list of paths of attachments for the reported attestation. Attachments can be files or directories. All attachments are compressed and uploaded to Kosli's evidence vault. | -| `-g`, `--commit` | string | [conditional] The git commit for which the attestation is associated to. Becomes required when reporting an attestation for an artifact before reporting it to Kosli. (defaulted in some CIs: [docs](/integrations/ci_cd) ). | +| `-g`, `--commit` | string | [conditional] The git commit for which the attestation is associated to. Becomes required when reporting an attestation for an artifact before reporting it to Kosli. (defaulted in some CIs: [docs](/integrations/ci_cd) ). If both `--commit` and `--repo-root` are left at their defaults and the commit cannot be read from the repository, a warning is printed and the attestation is sent without commit info. | | `--description` | string | [optional] attestation description | | `-D`, `--dry-run` | bool | [optional] Run in dry-run mode. When enabled, no data is sent to Kosli and the CLI exits with 0 exit code regardless of any errors. | | `-x`, `--exclude` | strings | [optional] The comma separated list of directories and files to exclude from fingerprinting. Can take glob patterns. Only applicable for `--artifact-type` dir. | diff --git a/internal/testHelpers/testHelpers.go b/internal/testHelpers/testHelpers.go index 534d7e486..558dff606 100644 --- a/internal/testHelpers/testHelpers.go +++ b/internal/testHelpers/testHelpers.go @@ -52,11 +52,18 @@ func GithubPRNumber() int { return 829 } +// CloneGitRepo clones url into cloneTo, which must already exist. func CloneGitRepo(url, cloneTo string) (*git.Repository, error) { + // osfs resolves symlinks in cloneTo but not in the ".git" path built from + // it, so resolve first to keep the two roots consistent. + resolvedCloneTo, err := filepath.EvalSymlinks(cloneTo) + if err != nil { + return nil, err + } // the repo worktree filesystem. It has to be osfs so that we can give it a path - fs := osfs.New(cloneTo) + fs := osfs.New(resolvedCloneTo) // the filesystem for git database - storerFS := osfs.New(filepath.Join(cloneTo, ".git")) + storerFS := osfs.New(filepath.Join(resolvedCloneTo, ".git")) storer := filesystem.NewStorage(storerFS, cache.NewObjectLRUDefault()) return git.Clone(storer, fs, &git.CloneOptions{URL: url}) }