From 1423e9e96cb7a3bd0c7d42d58e9d44a5dfbe8ef3 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:25:43 +0100 Subject: [PATCH 1/7] fix(attest): don't fail when a CI-defaulted --commit has no repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --commit is populated from CI environment variables (GITHUB_SHA, CI_COMMIT_SHA, BITBUCKET_COMMIT, ...) whether or not the user asked for it. The common attestation flow and begin trail then ran a git lookup because commitSHA was non-empty, so a CI job that has not checked out the repository failed with "failed to get commit info. failed to open git repository at .: repository does not exist" for a commit the user never requested. A commit that arrived from the CI default now warns and proceeds without commit info; a commit the user passed explicitly still errors, so a wrong --repo-root is not silently swallowed. An unresolvable commit in a shallow clone takes the same route, being the same surprise for the same reason. Doing nothing was not an option for affected users: the empty-value rule in refuseEmptyFlagValues rejects --commit "", so there is no way to opt out of the CI default from the command line. The two copies of the lookup are now one resolveCommitInfo, which is also the only level at which the CI default is testable — DefaultValue returns "" whenever KOSLI_TESTS is set, so no command-level test can produce an implicitly defaulted --commit. attest pr * and attest jira need the commit to do their work and dereferenced payload.Commit unguarded. That was unreachable while the empty-value rule blocked every route to a nil commit; soft-failing opens one, so both now report what is missing instead of panicking. Refs kosli-dev/server#6094 --- cmd/kosli/attestCustom.go | 1 + cmd/kosli/attestDecision.go | 1 + cmd/kosli/attestGeneric.go | 1 + cmd/kosli/attestJira.go | 5 + cmd/kosli/attestJunit.go | 1 + cmd/kosli/attestOverride.go | 1 + cmd/kosli/attestPRAzure.go | 1 + cmd/kosli/attestPRBitbucket.go | 1 + cmd/kosli/attestPRGithub.go | 1 + cmd/kosli/attestPRGitlab.go | 1 + cmd/kosli/attestSnyk.go | 1 + cmd/kosli/attestSonar.go | 1 + cmd/kosli/attestation.go | 29 +++- cmd/kosli/beginTrail.go | 9 +- cmd/kosli/commitInfoResolution_test.go | 126 ++++++++++++++++++ cmd/kosli/pullrequest.go | 4 + cmd/kosli/root.go | 4 +- .../testdata/output/docs/mintlify/snyk.md | 2 +- 18 files changed, 174 insertions(+), 16 deletions(-) create mode 100644 cmd/kosli/commitInfoResolution_test.go diff --git a/cmd/kosli/attestCustom.go b/cmd/kosli/attestCustom.go index 534de235e..4adb9b745 100644 --- a/cmd/kosli/attestCustom.go +++ b/cmd/kosli/attestCustom.go @@ -139,6 +139,7 @@ func newAttestCustomCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestDecision.go b/cmd/kosli/attestDecision.go index 8c7918f47..38e539d7f 100644 --- a/cmd/kosli/attestDecision.go +++ b/cmd/kosli/attestDecision.go @@ -135,6 +135,7 @@ func newAttestDecisionCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestGeneric.go b/cmd/kosli/attestGeneric.go index c5b3781c6..78fdb5f4b 100644 --- a/cmd/kosli/attestGeneric.go +++ b/cmd/kosli/attestGeneric.go @@ -133,6 +133,7 @@ func newAttestGenericCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestJira.go b/cmd/kosli/attestJira.go index 84556acc5..4ea5c5506 100644 --- a/cmd/kosli/attestJira.go +++ b/cmd/kosli/attestJira.go @@ -299,6 +299,7 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } @@ -345,6 +346,10 @@ func (o *attestJiraOptions) run(args []string) error { return err } + if o.payload.Commit == nil { + return fmt.Errorf("failed to get commit info, which is required to search for Jira issue keys. Pass --commit and point --repo-root at a repository containing it") + } + gv, err := gitview.New(o.srcRepoRoot) if err != nil { return err diff --git a/cmd/kosli/attestJunit.go b/cmd/kosli/attestJunit.go index 1bc1c0318..319d4bdcb 100644 --- a/cmd/kosli/attestJunit.go +++ b/cmd/kosli/attestJunit.go @@ -137,6 +137,7 @@ func newAttestJunitCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestOverride.go b/cmd/kosli/attestOverride.go index 9c794c3f4..66e1b9390 100644 --- a/cmd/kosli/attestOverride.go +++ b/cmd/kosli/attestOverride.go @@ -107,6 +107,7 @@ func newAttestOverrideCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestPRAzure.go b/cmd/kosli/attestPRAzure.go index 276f0fdbb..24216acc1 100644 --- a/cmd/kosli/attestPRAzure.go +++ b/cmd/kosli/attestPRAzure.go @@ -150,6 +150,7 @@ func newAttestAzurePRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") o.retriever = azUtils.NewAzureConfig(azureFlagsValues.Token, azureFlagsValues.OrgUrl, azureFlagsValues.Project, o.repoName) return o.run(args) diff --git a/cmd/kosli/attestPRBitbucket.go b/cmd/kosli/attestPRBitbucket.go index 7b36f84d8..0251f0ef4 100644 --- a/cmd/kosli/attestPRBitbucket.go +++ b/cmd/kosli/attestPRBitbucket.go @@ -171,6 +171,7 @@ func newAttestBitbucketPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") o.getRetriever().(*bbUtils.Config).Repository = o.repoName return o.run(args) }, diff --git a/cmd/kosli/attestPRGithub.go b/cmd/kosli/attestPRGithub.go index a7cf9bd8f..03c8947d3 100644 --- a/cmd/kosli/attestPRGithub.go +++ b/cmd/kosli/attestPRGithub.go @@ -145,6 +145,7 @@ func newAttestGithubPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, githubFlagsValues.Org, o.repoName, global.Debug) return o.run(args) diff --git a/cmd/kosli/attestPRGitlab.go b/cmd/kosli/attestPRGitlab.go index b595bf66f..016d4b283 100644 --- a/cmd/kosli/attestPRGitlab.go +++ b/cmd/kosli/attestPRGitlab.go @@ -145,6 +145,7 @@ func newAttestGitlabPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") // GitlabConfig.Repository is the short project name (CI_PROJECT_NAME); // combined with Org (CI_PROJECT_NAMESPACE) it forms the API ProjectID. // This is separate from repo_info.name, which uses the full CI_PROJECT_PATH. diff --git a/cmd/kosli/attestSnyk.go b/cmd/kosli/attestSnyk.go index 238aa2db1..dc108f08e 100644 --- a/cmd/kosli/attestSnyk.go +++ b/cmd/kosli/attestSnyk.go @@ -149,6 +149,7 @@ func newAttestSnykCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index 4fd2572cc..f54730800 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -219,6 +219,7 @@ func newAttestSonarCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") o.revisionExplicit = cmd.Flags().Changed("sonar-revision") return o.run(args) }, diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index e0d747156..8e5905e7e 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -56,6 +56,7 @@ type CommonAttestationOptions struct { repoProvider string repoURLExplicit bool repoNameExplicit bool + commitSHAExplicit bool } func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error { @@ -80,15 +81,10 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation } if o.commitSHA != "" { - gv, err := gitview.New(o.srcRepoRoot) + payload.Commit, err = resolveCommitInfo(o.srcRepoRoot, o.commitSHA, o.commitSHAExplicit, o.redactedCommitInfo) 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 } payload.GitRepoInfo, err = getGitRepoInfoFromEnvironment() @@ -117,6 +113,25 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation return err } +// resolveCommitInfo returns nil when git cannot supply the commit info and the +// commit was not asked for explicitly, so a CI-defaulted --commit does not fail +// the command in a job with no checked-out repository (#6094). +func resolveCommitInfo(srcRepoRoot, commitSHA string, explicit bool, redactedCommitInfo []string) (*gitview.BasicCommitInfo, error) { + gv, err := gitview.New(srcRepoRoot) + if err == nil { + var commitInfo *gitview.CommitInfo + commitInfo, err = gv.GetCommitInfoFromCommitSHA(commitSHA, false, redactedCommitInfo) + if err == nil { + return &commitInfo.BasicCommitInfo, nil + } + } + if explicit { + return nil, fmt.Errorf("failed to get commit info. %s", err) + } + logger.Warn("attesting without commit info: --commit defaulted to %s from the CI environment, but %s. Point --repo-root at a repository containing that commit to attach it.", commitSHA, err.Error()) + 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/beginTrail.go b/cmd/kosli/beginTrail.go index 8a796b34f..b281c02b4 100644 --- a/cmd/kosli/beginTrail.go +++ b/cmd/kosli/beginTrail.go @@ -50,6 +50,7 @@ type beginTrailOptions struct { repoURL string repoProvider string repoNameExplicit bool + commitSHAExplicit bool } type TrailPayload struct { @@ -85,6 +86,7 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { o.repoNameExplicit = cmd.Flags().Changed("repository") + o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } @@ -128,15 +130,10 @@ func (o *beginTrailOptions) run(args []string) error { } if o.commitSHA != "" { - gv, err := gitview.New(o.srcRepoRoot) + o.payload.Commit, err = resolveCommitInfo(o.srcRepoRoot, o.commitSHA, o.commitSHAExplicit, o.redactedCommitInfo) 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/commitInfoResolution_test.go b/cmd/kosli/commitInfoResolution_test.go new file mode 100644 index 000000000..092ff8de9 --- /dev/null +++ b/cmd/kosli/commitInfoResolution_test.go @@ -0,0 +1,126 @@ +package main + +import ( + "fmt" + "testing" + + "github.com/go-git/go-git/v5" + "github.com/stretchr/testify/suite" +) + +// CommitInfoResolutionTestSuite guards that a --commit which was defaulted from +// the CI environment does not fail the command when git cannot supply its info, +// while an explicitly passed --commit still does. +// +// The production trigger (a CI-defaulted --commit in a job with no checked-out +// repository) cannot be reproduced through the command harness, because +// DefaultValue returns "" whenever KOSLI_TESTS is set. resolveCommitInfo is +// therefore exercised directly, and the command cases below guard only that +// each command assigns commitSHAExplicit. +type CommitInfoResolutionTestSuite struct { + suite.Suite + headHash string + defaultKosliArguments string +} + +func (suite *CommitInfoResolutionTestSuite) 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" +} + +func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoWithoutRepository() { + const noRepo = "testdata" + + info, err := resolveCommitInfo(noRepo, suite.headHash, false, []string{}) + suite.Require().NoError(err, "a CI-defaulted commit must not fail when there is no repository") + suite.Nil(info) + + _, err = resolveCommitInfo(noRepo, suite.headHash, true, []string{}) + suite.Require().Error(err, "an explicit --commit must still fail when there is no repository") + suite.Contains(err.Error(), "repository does not exist") +} + +func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoWithUnresolvableCommit() { + // A well-formed SHA that is not in this repository, as in a shallow clone. + const absentSHA = "0d4c1e1b7f5c2a9e8b3d6f0a1c4e7b2d5a8f3c60" + + info, err := resolveCommitInfo("../..", absentSHA, false, []string{}) + suite.Require().NoError(err, "a CI-defaulted commit must not fail when it cannot be resolved") + suite.Nil(info) + + _, err = resolveCommitInfo("../..", absentSHA, true, []string{}) + suite.Require().Error(err, "an explicit --commit must still fail when it cannot be resolved") +} + +func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoSucceeds() { + info, err := resolveCommitInfo("../..", suite.headHash, false, []string{}) + suite.Require().NoError(err) + suite.Require().NotNil(info) + suite.Equal(suite.headHash, info.Sha1) +} + +func (suite *CommitInfoResolutionTestSuite) TestExplicitCommitWiring() { + tests := []cmdTestCase{ + { + wantError: true, + name: "attest generic: an explicit --commit fails when --repo-root has no repository", + cmd: fmt.Sprintf("attest generic --fingerprint 7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --flow f --trail t --commit %s --repo-root testdata%s", suite.headHash, suite.defaultKosliArguments), + goldenRegex: "Error: failed to get commit info\\. .*repository does not exist\n", + }, + { + wantError: true, + name: "begin trail: an explicit --commit fails when --repo-root has no repository", + cmd: fmt.Sprintf("begin trail t --flow f --commit %s --repo-root testdata%s", suite.headHash, suite.defaultKosliArguments), + goldenRegex: "Error: failed to get commit info\\. .*repository does not exist\n", + }, + } + runTestCmd(suite.T(), tests) +} + +// commitRequiredOptions builds the shared attestation options for a command run +// whose --commit came from the CI default and cannot be resolved, which is the +// only way payload.Commit reaches these commands as nil. +func (suite *CommitInfoResolutionTestSuite) commitRequiredOptions() *CommonAttestationOptions { + return &CommonAttestationOptions{ + fingerprintOptions: &fingerprintOptions{}, + attestationNameTemplate: "foo", + flowName: "f", + trailName: "t", + commitSHA: suite.headHash, + srcRepoRoot: "testdata", + commitSHAExplicit: false, + } +} + +func (suite *CommitInfoResolutionTestSuite) TestCommandsNeedingCommitReportIt() { + pr := &attestPROptions{ + CommonAttestationOptions: suite.commitRequiredOptions(), + payload: PRAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}}, + } + err := pr.run([]string{}) + suite.Require().Error(err) + suite.Contains(err.Error(), "required to find pull requests") + + jira := &attestJiraOptions{ + CommonAttestationOptions: suite.commitRequiredOptions(), + payload: JiraAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}}, + } + err = jira.run([]string{}) + suite.Require().Error(err) + suite.Contains(err.Error(), "required to search for Jira issue keys") +} + +func TestCommitInfoResolutionTestSuite(t *testing.T) { + suite.Run(t, new(CommitInfoResolutionTestSuite)) +} diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index 5f1c329d5..44c4e2d78 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -38,6 +38,10 @@ func (o *attestPROptions) run(args []string) error { return err } + if o.payload.Commit == nil { + return fmt.Errorf("failed to get commit info, which is required to find pull requests. Pass --commit and point --repo-root at a repository containing it") + } + label := "" o.payload.GitProvider, label = o.getRetriever().ProviderAndLabel() diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 32c689145..3a88cc24e 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 ). When it is defaulted from the CI environment and no git repository is available at --repo-root, 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 ). When it is defaulted from the CI environment and no git repository is available at --repo-root, 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..eca4495bd 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) ). When it is defaulted from the CI environment and no git repository is available at `--repo-root`, 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. | From c601e766ad91dc3bf690ddaa8f608a3cecdb0846 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:25:50 +0100 Subject: [PATCH 2/7] test(helpers): resolve symlinks in CloneGitRepo before cloning On macOS os.MkdirTemp returns a path under /var/folders, which is a symlink to /private/var/folders. osfs resolves the symlink for the worktree root but leaves the caller's unresolved path in place for the git dir, so go-git sees a git dir that is not ".git" relative to the worktree, decides the repository lives elsewhere, and tries to write a "gitdir:" file over the directory it just created: open /private/var/folders/.../testDir123/.git: is a directory InitializeGitRepo in the same file already resolves the path for exactly this reason. CloneGitRepo did not, so AttestGitlabPRCommandTestSuite and AttestBitbucketPRCommandTestSuite fail in SetupTest on any machine whose temp dir is symlinked. --- internal/testHelpers/testHelpers.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/testHelpers/testHelpers.go b/internal/testHelpers/testHelpers.go index 534d7e486..fa53bae96 100644 --- a/internal/testHelpers/testHelpers.go +++ b/internal/testHelpers/testHelpers.go @@ -53,10 +53,15 @@ func GithubPRNumber() int { } func CloneGitRepo(url, cloneTo string) (*git.Repository, error) { + // Resolved for the same reason as in InitializeGitRepo below. + 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}) } From fc209f52c583fb66a6a812e4ba3e0586c8c108a4 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:25:54 +0100 Subject: [PATCH 3/7] fix(attest): fail on an explicit --repo-root and cover the CI-defaulted commit end to end A --commit defaulted from the CI environment is only downgraded to a warning when --repo-root was left at its default too: a user who pointed at a repository that is not there asked for something and should hear that it failed. The check for whether a flag was passed now reads the command's flag set kept by addAttestationFlags, so no attest command has to remember to capture it. attest pullrequest * and attest jira cannot do their work without the commit, so they name that need in one error rather than warning and then failing. The CI default is reachable through the command harness by unsetting KOSLI_TESTS around the run, as TestDefaultValue already does, so the fix is now exercised end to end: the command exits 0, warns, and sends no git_commit_info. Refs kosli-dev/server#6094, kosli-dev/server#5615 --- cmd/kosli/attestCustom.go | 1 - cmd/kosli/attestDecision.go | 1 - cmd/kosli/attestGeneric.go | 1 - cmd/kosli/attestJira.go | 6 +- cmd/kosli/attestJunit.go | 1 - cmd/kosli/attestOverride.go | 1 - cmd/kosli/attestPRAzure.go | 1 - cmd/kosli/attestPRBitbucket.go | 1 - cmd/kosli/attestPRGithub.go | 1 - cmd/kosli/attestPRGitlab.go | 1 - cmd/kosli/attestSnyk.go | 1 - cmd/kosli/attestSonar.go | 1 - cmd/kosli/attestation.go | 67 +++++++-- cmd/kosli/attestationCommitInfo_test.go | 174 ++++++++++++++++++++++++ cmd/kosli/beginTrail.go | 14 +- cmd/kosli/commitInfoResolution_test.go | 126 ----------------- cmd/kosli/flags.go | 1 + cmd/kosli/pullrequest.go | 5 +- 18 files changed, 244 insertions(+), 160 deletions(-) create mode 100644 cmd/kosli/attestationCommitInfo_test.go delete mode 100644 cmd/kosli/commitInfoResolution_test.go diff --git a/cmd/kosli/attestCustom.go b/cmd/kosli/attestCustom.go index 4adb9b745..534de235e 100644 --- a/cmd/kosli/attestCustom.go +++ b/cmd/kosli/attestCustom.go @@ -139,7 +139,6 @@ func newAttestCustomCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestDecision.go b/cmd/kosli/attestDecision.go index 38e539d7f..8c7918f47 100644 --- a/cmd/kosli/attestDecision.go +++ b/cmd/kosli/attestDecision.go @@ -135,7 +135,6 @@ func newAttestDecisionCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestGeneric.go b/cmd/kosli/attestGeneric.go index 78fdb5f4b..c5b3781c6 100644 --- a/cmd/kosli/attestGeneric.go +++ b/cmd/kosli/attestGeneric.go @@ -133,7 +133,6 @@ func newAttestGenericCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestJira.go b/cmd/kosli/attestJira.go index 4ea5c5506..c3849f224 100644 --- a/cmd/kosli/attestJira.go +++ b/cmd/kosli/attestJira.go @@ -299,7 +299,6 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } @@ -331,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 @@ -346,10 +346,6 @@ func (o *attestJiraOptions) run(args []string) error { return err } - if o.payload.Commit == nil { - return fmt.Errorf("failed to get commit info, which is required to search for Jira issue keys. Pass --commit and point --repo-root at a repository containing it") - } - gv, err := gitview.New(o.srcRepoRoot) if err != nil { return err diff --git a/cmd/kosli/attestJunit.go b/cmd/kosli/attestJunit.go index 319d4bdcb..1bc1c0318 100644 --- a/cmd/kosli/attestJunit.go +++ b/cmd/kosli/attestJunit.go @@ -137,7 +137,6 @@ func newAttestJunitCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestOverride.go b/cmd/kosli/attestOverride.go index 66e1b9390..9c794c3f4 100644 --- a/cmd/kosli/attestOverride.go +++ b/cmd/kosli/attestOverride.go @@ -107,7 +107,6 @@ func newAttestOverrideCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestPRAzure.go b/cmd/kosli/attestPRAzure.go index 24216acc1..276f0fdbb 100644 --- a/cmd/kosli/attestPRAzure.go +++ b/cmd/kosli/attestPRAzure.go @@ -150,7 +150,6 @@ func newAttestAzurePRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") o.retriever = azUtils.NewAzureConfig(azureFlagsValues.Token, azureFlagsValues.OrgUrl, azureFlagsValues.Project, o.repoName) return o.run(args) diff --git a/cmd/kosli/attestPRBitbucket.go b/cmd/kosli/attestPRBitbucket.go index 0251f0ef4..7b36f84d8 100644 --- a/cmd/kosli/attestPRBitbucket.go +++ b/cmd/kosli/attestPRBitbucket.go @@ -171,7 +171,6 @@ func newAttestBitbucketPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") o.getRetriever().(*bbUtils.Config).Repository = o.repoName return o.run(args) }, diff --git a/cmd/kosli/attestPRGithub.go b/cmd/kosli/attestPRGithub.go index 03c8947d3..a7cf9bd8f 100644 --- a/cmd/kosli/attestPRGithub.go +++ b/cmd/kosli/attestPRGithub.go @@ -145,7 +145,6 @@ func newAttestGithubPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, githubFlagsValues.Org, o.repoName, global.Debug) return o.run(args) diff --git a/cmd/kosli/attestPRGitlab.go b/cmd/kosli/attestPRGitlab.go index 016d4b283..b595bf66f 100644 --- a/cmd/kosli/attestPRGitlab.go +++ b/cmd/kosli/attestPRGitlab.go @@ -145,7 +145,6 @@ func newAttestGitlabPRCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") // GitlabConfig.Repository is the short project name (CI_PROJECT_NAME); // combined with Org (CI_PROJECT_NAMESPACE) it forms the API ProjectID. // This is separate from repo_info.name, which uses the full CI_PROJECT_PATH. diff --git a/cmd/kosli/attestSnyk.go b/cmd/kosli/attestSnyk.go index dc108f08e..238aa2db1 100644 --- a/cmd/kosli/attestSnyk.go +++ b/cmd/kosli/attestSnyk.go @@ -149,7 +149,6 @@ func newAttestSnykCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") return o.run(args) }, } diff --git a/cmd/kosli/attestSonar.go b/cmd/kosli/attestSonar.go index f54730800..4fd2572cc 100644 --- a/cmd/kosli/attestSonar.go +++ b/cmd/kosli/attestSonar.go @@ -219,7 +219,6 @@ func newAttestSonarCmd(out io.Writer) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") o.revisionExplicit = cmd.Flags().Changed("sonar-revision") return o.run(args) }, diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index 8e5905e7e..ab1a4a991 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,7 +57,16 @@ type CommonAttestationOptions struct { repoProvider string repoURLExplicit bool repoNameExplicit bool - commitSHAExplicit bool + // flags is the command's flag set, kept so run can tell a passed flag from + // a defaulted one after parsing. + flags *pflag.FlagSet + // commitRequiredFor names what the command cannot do without the commit, + // e.g. "find pull requests". Empty when commit info is optional. + commitRequiredFor string +} + +func (o *CommonAttestationOptions) flagChanged(name string) bool { + return o.flags != nil && o.flags.Changed(name) } func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error { @@ -81,10 +91,19 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation } if o.commitSHA != "" { - payload.Commit, err = resolveCommitInfo(o.srcRepoRoot, o.commitSHA, o.commitSHAExplicit, o.redactedCommitInfo) + payload.Commit, err = commitInfoRequest{ + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + commitExplicit: o.flagChanged("commit"), + repoRootExplicit: o.flagChanged("repo-root"), + requiredFor: o.commitRequiredFor, + }.resolve() if err != nil { return err } + } 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() @@ -113,22 +132,48 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation return err } -// resolveCommitInfo returns nil when git cannot supply the commit info and the -// commit was not asked for explicitly, so a CI-defaulted --commit does not fail -// the command in a job with no checked-out repository (#6094). -func resolveCommitInfo(srcRepoRoot, commitSHA string, explicit bool, redactedCommitInfo []string) (*gitview.BasicCommitInfo, error) { - gv, err := gitview.New(srcRepoRoot) +// commitInfoRequest is one attempt to read the commit info for a payload from +// the repository at repoRoot. +type commitInfoRequest struct { + repoRoot string + sha string + redacted []string + // commitExplicit is false when --commit was defaulted from the CI environment. + commitExplicit bool + // repoRootExplicit is true when --repo-root was passed rather than left at ".". + repoRootExplicit bool + // requiredFor names what the command cannot do without the commit; empty + // when commit info is optional. + requiredFor string +} + +// resolve returns nil, nil when the lookup fails but neither the commit nor +// the repository was asked for: a --commit defaulted from CI in a job with no +// checked-out repository must not fail the command (kosli-dev/server#6094, +// kosli-dev/server#5615). A commit that is not in the repository, as in a +// shallow clone, is the same surprise for the same reason and takes the same +// route. Anything asked for explicitly, or needed by the command, still fails. +func (r commitInfoRequest) resolve() (*gitview.BasicCommitInfo, error) { + gv, err := gitview.New(r.repoRoot) if err == nil { var commitInfo *gitview.CommitInfo - commitInfo, err = gv.GetCommitInfoFromCommitSHA(commitSHA, false, redactedCommitInfo) + commitInfo, err = gv.GetCommitInfoFromCommitSHA(r.sha, false, r.redacted) if err == nil { return &commitInfo.BasicCommitInfo, nil } } - if explicit { - return nil, fmt.Errorf("failed to get commit info. %s", err) + + origin := "--commit " + r.sha + if !r.commitExplicit { + origin += " (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", origin, 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", origin, err) } - logger.Warn("attesting without commit info: --commit defaulted to %s from the CI environment, but %s. Point --repo-root at a repository containing that commit to attach it.", commitSHA, err.Error()) + logger.Warn("proceeding without commit info: %s could not be read: %s. 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.", origin, err) return nil, nil } diff --git a/cmd/kosli/attestationCommitInfo_test.go b/cmd/kosli/attestationCommitInfo_test.go new file mode 100644 index 000000000..5a5a18067 --- /dev/null +++ b/cmd/kosli/attestationCommitInfo_test.go @@ -0,0 +1,174 @@ +package main + +import ( + "os" + "testing" + + "github.com/go-git/go-git/v5" + "github.com/stretchr/testify/suite" +) + +// AttestationCommitInfoTestSuite guards how a failed commit lookup is reported. +// A --commit defaulted from the CI environment must not fail a command that +// asked for no repository (kosli-dev/server#6094, kosli-dev/server#5615), while +// anything asked for explicitly, or needed by the command, still fails. +// +// The CI default exists only while KOSLI_TESTS is unset, because DefaultValue +// returns "" under it. inCI unsets it around a command run, as TestDefaultValue +// does, and simulates a GitHub Actions job whose GITHUB_SHA is the given commit. +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) 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) +} + +// The commands that do their work from the commit get one error naming what +// needs it, not a warning followed by a nil dereference. +func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() { + suite.T().Chdir(suite.T().TempDir()) + suite.inCI(suite.headHash, func() { + for cmd, need := range map[string]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(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(), "The commit is required to "+need, cmd) + suite.NotContains(out, "[warning] proceeding without commit info", cmd) + } + }) +} + +// RequireFlags keeps --commit non-empty for these commands, so this guards the +// nil dereference that would follow if that ever 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 b281c02b4..af09d6926 100644 --- a/cmd/kosli/beginTrail.go +++ b/cmd/kosli/beginTrail.go @@ -50,7 +50,8 @@ type beginTrailOptions struct { repoURL string repoProvider string repoNameExplicit bool - commitSHAExplicit bool + commitExplicit bool + repoRootExplicit bool } type TrailPayload struct { @@ -86,7 +87,8 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitSHAExplicit = cmd.Flags().Changed("commit") + o.commitExplicit = cmd.Flags().Changed("commit") + o.repoRootExplicit = cmd.Flags().Changed("repo-root") return o.run(args) }, } @@ -130,7 +132,13 @@ func (o *beginTrailOptions) run(args []string) error { } if o.commitSHA != "" { - o.payload.Commit, err = resolveCommitInfo(o.srcRepoRoot, o.commitSHA, o.commitSHAExplicit, o.redactedCommitInfo) + o.payload.Commit, err = commitInfoRequest{ + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + commitExplicit: o.commitExplicit, + repoRootExplicit: o.repoRootExplicit, + }.resolve() if err != nil { return err } diff --git a/cmd/kosli/commitInfoResolution_test.go b/cmd/kosli/commitInfoResolution_test.go deleted file mode 100644 index 092ff8de9..000000000 --- a/cmd/kosli/commitInfoResolution_test.go +++ /dev/null @@ -1,126 +0,0 @@ -package main - -import ( - "fmt" - "testing" - - "github.com/go-git/go-git/v5" - "github.com/stretchr/testify/suite" -) - -// CommitInfoResolutionTestSuite guards that a --commit which was defaulted from -// the CI environment does not fail the command when git cannot supply its info, -// while an explicitly passed --commit still does. -// -// The production trigger (a CI-defaulted --commit in a job with no checked-out -// repository) cannot be reproduced through the command harness, because -// DefaultValue returns "" whenever KOSLI_TESTS is set. resolveCommitInfo is -// therefore exercised directly, and the command cases below guard only that -// each command assigns commitSHAExplicit. -type CommitInfoResolutionTestSuite struct { - suite.Suite - headHash string - defaultKosliArguments string -} - -func (suite *CommitInfoResolutionTestSuite) 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" -} - -func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoWithoutRepository() { - const noRepo = "testdata" - - info, err := resolveCommitInfo(noRepo, suite.headHash, false, []string{}) - suite.Require().NoError(err, "a CI-defaulted commit must not fail when there is no repository") - suite.Nil(info) - - _, err = resolveCommitInfo(noRepo, suite.headHash, true, []string{}) - suite.Require().Error(err, "an explicit --commit must still fail when there is no repository") - suite.Contains(err.Error(), "repository does not exist") -} - -func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoWithUnresolvableCommit() { - // A well-formed SHA that is not in this repository, as in a shallow clone. - const absentSHA = "0d4c1e1b7f5c2a9e8b3d6f0a1c4e7b2d5a8f3c60" - - info, err := resolveCommitInfo("../..", absentSHA, false, []string{}) - suite.Require().NoError(err, "a CI-defaulted commit must not fail when it cannot be resolved") - suite.Nil(info) - - _, err = resolveCommitInfo("../..", absentSHA, true, []string{}) - suite.Require().Error(err, "an explicit --commit must still fail when it cannot be resolved") -} - -func (suite *CommitInfoResolutionTestSuite) TestResolveCommitInfoSucceeds() { - info, err := resolveCommitInfo("../..", suite.headHash, false, []string{}) - suite.Require().NoError(err) - suite.Require().NotNil(info) - suite.Equal(suite.headHash, info.Sha1) -} - -func (suite *CommitInfoResolutionTestSuite) TestExplicitCommitWiring() { - tests := []cmdTestCase{ - { - wantError: true, - name: "attest generic: an explicit --commit fails when --repo-root has no repository", - cmd: fmt.Sprintf("attest generic --fingerprint 7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --flow f --trail t --commit %s --repo-root testdata%s", suite.headHash, suite.defaultKosliArguments), - goldenRegex: "Error: failed to get commit info\\. .*repository does not exist\n", - }, - { - wantError: true, - name: "begin trail: an explicit --commit fails when --repo-root has no repository", - cmd: fmt.Sprintf("begin trail t --flow f --commit %s --repo-root testdata%s", suite.headHash, suite.defaultKosliArguments), - goldenRegex: "Error: failed to get commit info\\. .*repository does not exist\n", - }, - } - runTestCmd(suite.T(), tests) -} - -// commitRequiredOptions builds the shared attestation options for a command run -// whose --commit came from the CI default and cannot be resolved, which is the -// only way payload.Commit reaches these commands as nil. -func (suite *CommitInfoResolutionTestSuite) commitRequiredOptions() *CommonAttestationOptions { - return &CommonAttestationOptions{ - fingerprintOptions: &fingerprintOptions{}, - attestationNameTemplate: "foo", - flowName: "f", - trailName: "t", - commitSHA: suite.headHash, - srcRepoRoot: "testdata", - commitSHAExplicit: false, - } -} - -func (suite *CommitInfoResolutionTestSuite) TestCommandsNeedingCommitReportIt() { - pr := &attestPROptions{ - CommonAttestationOptions: suite.commitRequiredOptions(), - payload: PRAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}}, - } - err := pr.run([]string{}) - suite.Require().Error(err) - suite.Contains(err.Error(), "required to find pull requests") - - jira := &attestJiraOptions{ - CommonAttestationOptions: suite.commitRequiredOptions(), - payload: JiraAttestationPayload{CommonAttestationPayload: &CommonAttestationPayload{}}, - } - err = jira.run([]string{}) - suite.Require().Error(err) - suite.Contains(err.Error(), "required to search for Jira issue keys") -} - -func TestCommitInfoResolutionTestSuite(t *testing.T) { - suite.Run(t, new(CommitInfoResolutionTestSuite)) -} 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 44c4e2d78..e78da1cf3 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -33,15 +33,12 @@ 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 } - if o.payload.Commit == nil { - return fmt.Errorf("failed to get commit info, which is required to find pull requests. Pass --commit and point --repo-root at a repository containing it") - } - label := "" o.payload.GitProvider, label = o.getRetriever().ProviderAndLabel() From 3b73508eded3103aadc6bc58d9c5e4ab459dc127 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:25:57 +0100 Subject: [PATCH 4/7] docs(attest): say when --commit is dropped with a warning The help for --commit on the attest commands and begin trail now states the rule: only when both --commit and --repo-root are left at their defaults is a commit that cannot be read downgraded to a warning. Refs kosli-dev/server#6094 --- cmd/kosli/root.go | 4 ++-- cmd/kosli/testdata/output/docs/mintlify/snyk.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/kosli/root.go b/cmd/kosli/root.go index 3a88cc24e..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 ). When it is defaulted from the CI environment and no git repository is available at --repo-root, the attestation is sent without commit info." + 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 unset ). When it is defaulted from the CI environment and no git repository is available at --repo-root, the trail is begun without commit info." + 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 eca4495bd..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) ). When it is defaulted from the CI environment and no git repository is available at `--repo-root`, the attestation is sent without commit info. | +| `-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. | From f8964ef887813dd4c632e2ee1ed7fafca2b2fb15 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:26:02 +0100 Subject: [PATCH 5/7] chore: note CloneGitRepo's precondition and the commit rules in the attest skill CloneGitRepo resolves symlinks before cloning, so cloneTo has to exist; say so on the helper. The new-command attest archetype explains why --commit and --repo-root are not captured in RunE and when to set commitRequiredFor. --- .claude/skills/new-command/references/archetype-attest.md | 4 +++- internal/testHelpers/testHelpers.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.claude/skills/new-command/references/archetype-attest.md b/.claude/skills/new-command/references/archetype-attest.md index fdd4b5862..1930aa06b 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, so a `--commit` defaulted from CI is downgraded to a warning when the repository is missing, while a passed one still fails. +- If the command cannot do its work without the commit (as `attest pullrequest *` and `attest jira` cannot), set `o.commitRequiredFor = ""` in `run` before delegating so a failed lookup is one error naming that need. **`run` method** - Build the URL: `url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "")`. diff --git a/internal/testHelpers/testHelpers.go b/internal/testHelpers/testHelpers.go index fa53bae96..0481e4590 100644 --- a/internal/testHelpers/testHelpers.go +++ b/internal/testHelpers/testHelpers.go @@ -52,8 +52,10 @@ func GithubPRNumber() int { return 829 } +// CloneGitRepo clones url into cloneTo, which must already exist. func CloneGitRepo(url, cloneTo string) (*git.Repository, error) { - // Resolved for the same reason as in InitializeGitRepo below. + // Resolved for the same reason as in InitializeGitRepo below. EvalSymlinks + // fails on a missing path, hence the precondition. resolvedCloneTo, err := filepath.EvalSymlinks(cloneTo) if err != nil { return nil, err From a8a2cbe2c07e2bc425e62fb96c09dc8c02af5574 Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:26:05 +0100 Subject: [PATCH 6/7] style(attest): keep only the comments that add information Drop comments that restate a name or duplicate a sibling field, and trim the rest to the one fact the code cannot show: why a flag set sits on the options, what a false commitExplicit means, why resolve returns nil, nil, and why CloneGitRepo resolves symlinks first. --- .../references/archetype-attest.md | 4 +-- cmd/kosli/attestation.go | 27 +++++++------------ cmd/kosli/attestationCommitInfo_test.go | 18 +++++-------- internal/testHelpers/testHelpers.go | 4 +-- 4 files changed, 20 insertions(+), 33 deletions(-) diff --git a/.claude/skills/new-command/references/archetype-attest.md b/.claude/skills/new-command/references/archetype-attest.md index 1930aa06b..7c85b6886 100644 --- a/.claude/skills/new-command/references/archetype-attest.md +++ b/.claude/skills/new-command/references/archetype-attest.md @@ -54,8 +54,8 @@ Canonical example: `cmd/kosli/attestCustom.go` — read it in full and adapt. **`RunE`** - 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, so a `--commit` defaulted from CI is downgraded to a warning when the repository is missing, while a passed one still fails. -- If the command cannot do its work without the commit (as `attest pullrequest *` and `attest jira` cannot), set `o.commitRequiredFor = ""` in `run` before delegating so a failed lookup is one error naming that need. +- 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/attestation.go b/cmd/kosli/attestation.go index ab1a4a991..da2f911c2 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -57,11 +57,10 @@ type CommonAttestationOptions struct { repoProvider string repoURLExplicit bool repoNameExplicit bool - // flags is the command's flag set, kept so run can tell a passed flag from - // a defaulted one after parsing. + // flags lets run tell a passed flag from a defaulted one. flags *pflag.FlagSet - // commitRequiredFor names what the command cannot do without the commit, - // e.g. "find pull requests". Empty when commit info is optional. + // commitRequiredFor completes "the commit is required to ..."; empty when + // commit info is optional. commitRequiredFor string } @@ -132,27 +131,21 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation return err } -// commitInfoRequest is one attempt to read the commit info for a payload from -// the repository at repoRoot. type commitInfoRequest struct { repoRoot string sha string redacted []string // commitExplicit is false when --commit was defaulted from the CI environment. - commitExplicit bool - // repoRootExplicit is true when --repo-root was passed rather than left at ".". + commitExplicit bool repoRootExplicit bool - // requiredFor names what the command cannot do without the commit; empty - // when commit info is optional. - requiredFor string + requiredFor string } -// resolve returns nil, nil when the lookup fails but neither the commit nor -// the repository was asked for: a --commit defaulted from CI in a job with no -// checked-out repository must not fail the command (kosli-dev/server#6094, -// kosli-dev/server#5615). A commit that is not in the repository, as in a -// shallow clone, is the same surprise for the same reason and takes the same -// route. Anything asked for explicitly, or needed by the command, still fails. +// 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) { gv, err := gitview.New(r.repoRoot) if err == nil { diff --git a/cmd/kosli/attestationCommitInfo_test.go b/cmd/kosli/attestationCommitInfo_test.go index 5a5a18067..6f7fcc7a5 100644 --- a/cmd/kosli/attestationCommitInfo_test.go +++ b/cmd/kosli/attestationCommitInfo_test.go @@ -8,14 +8,10 @@ import ( "github.com/stretchr/testify/suite" ) -// AttestationCommitInfoTestSuite guards how a failed commit lookup is reported. -// A --commit defaulted from the CI environment must not fail a command that -// asked for no repository (kosli-dev/server#6094, kosli-dev/server#5615), while -// anything asked for explicitly, or needed by the command, still fails. -// -// The CI default exists only while KOSLI_TESTS is unset, because DefaultValue -// returns "" under it. inCI unsets it around a command run, as TestDefaultValue -// does, and simulates a GitHub Actions job whose GITHUB_SHA is the given commit. +// 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 @@ -138,8 +134,6 @@ func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitIsAttached() { runTestCmd(suite.T(), tests) } -// The commands that do their work from the commit get one error naming what -// needs it, not a warning followed by a nil dereference. func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() { suite.T().Chdir(suite.T().TempDir()) suite.inCI(suite.headHash, func() { @@ -156,8 +150,8 @@ func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() }) } -// RequireFlags keeps --commit non-empty for these commands, so this guards the -// nil dereference that would follow if that ever changed. +// 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{}, diff --git a/internal/testHelpers/testHelpers.go b/internal/testHelpers/testHelpers.go index 0481e4590..558dff606 100644 --- a/internal/testHelpers/testHelpers.go +++ b/internal/testHelpers/testHelpers.go @@ -54,8 +54,8 @@ func GithubPRNumber() int { // CloneGitRepo clones url into cloneTo, which must already exist. func CloneGitRepo(url, cloneTo string) (*git.Repository, error) { - // Resolved for the same reason as in InitializeGitRepo below. EvalSymlinks - // fails on a missing path, hence the precondition. + // 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 From 07848ba582b85ebcae645544b0283b05e60f2a2d Mon Sep 17 00:00:00 2001 From: Marko Bevc Date: Thu, 17 Sep 2026 19:26:09 +0100 Subject: [PATCH 7/7] fix(attest): treat repo-root at its default as unset, and unify explicit-flag detection A config or env value for --repo-root marked the flag Changed even when it equaled the "." default, because bindFlags applies it with Flags().Set() regardless of value. That made KOSLI_REPO_ROOT=. or a config-file repo-root: "." hard-fail a CI-defaulted commit instead of warning, contradicting the help text. repoRootExplicit now also requires the value to differ from ".". begin trail kept its own commitExplicit/repoRootExplicit booleans because it doesn't go through addAttestationFlags. commitInfoRequest now takes the flag set directly and derives both from it, so there is one place, not two, that knows how "explicit" is decided. Adds coverage for the KOSLI_COMMIT env route (explicit, hard-fails) and the KOSLI_REPO_ROOT=. case (still warns), and converts a map-based test loop to a slice for deterministic failure output. Addresses review feedback on kosli-dev/cli#1202. --- cmd/kosli/attestation.go | 71 ++++++++++++++----------- cmd/kosli/attestationCommitInfo_test.go | 39 +++++++++++--- cmd/kosli/beginTrail.go | 18 +++---- 3 files changed, 81 insertions(+), 47 deletions(-) diff --git a/cmd/kosli/attestation.go b/cmd/kosli/attestation.go index da2f911c2..bc1605214 100644 --- a/cmd/kosli/attestation.go +++ b/cmd/kosli/attestation.go @@ -64,10 +64,6 @@ type CommonAttestationOptions struct { commitRequiredFor string } -func (o *CommonAttestationOptions) flagChanged(name string) bool { - return o.flags != nil && o.flags.Changed(name) -} - func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error { var err error @@ -91,12 +87,11 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation if o.commitSHA != "" { payload.Commit, err = commitInfoRequest{ - repoRoot: o.srcRepoRoot, - sha: o.commitSHA, - redacted: o.redactedCommitInfo, - commitExplicit: o.flagChanged("commit"), - repoRootExplicit: o.flagChanged("repo-root"), - requiredFor: o.commitRequiredFor, + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + flags: o.flags, + requiredFor: o.commitRequiredFor, }.resolve() if err != nil { return err @@ -132,13 +127,32 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation } type commitInfoRequest struct { - repoRoot string - sha string - redacted []string - // commitExplicit is false when --commit was defaulted from the CI environment. - commitExplicit bool - repoRootExplicit bool - requiredFor string + 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 @@ -147,26 +161,23 @@ type commitInfoRequest struct { // (kosli-dev/server#6094). An unresolvable commit, as in a shallow clone, // deliberately takes the same route. func (r commitInfoRequest) resolve() (*gitview.BasicCommitInfo, error) { - gv, err := gitview.New(r.repoRoot) + commitInfo, err := r.lookup() if err == nil { - var commitInfo *gitview.CommitInfo - commitInfo, err = gv.GetCommitInfoFromCommitSHA(r.sha, false, r.redacted) - if err == nil { - return &commitInfo.BasicCommitInfo, nil - } + return &commitInfo.BasicCommitInfo, nil } - origin := "--commit " + r.sha - if !r.commitExplicit { - origin += " (defaulted from the CI environment)" + 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", origin, 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", origin, err) + 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. 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.", origin, 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 } diff --git a/cmd/kosli/attestationCommitInfo_test.go b/cmd/kosli/attestationCommitInfo_test.go index 6f7fcc7a5..018773dab 100644 --- a/cmd/kosli/attestationCommitInfo_test.go +++ b/cmd/kosli/attestationCommitInfo_test.go @@ -87,6 +87,29 @@ func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitNotInRepositor }) } +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")} { @@ -137,15 +160,15 @@ func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitIsAttached() { func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() { suite.T().Chdir(suite.T().TempDir()) suite.inCI(suite.headHash, func() { - for cmd, need := range map[string]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", + 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(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(), "The commit is required to "+need, cmd) - suite.NotContains(out, "[warning] proceeding without commit info", cmd) + _, 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) } }) } diff --git a/cmd/kosli/beginTrail.go b/cmd/kosli/beginTrail.go index af09d6926..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,8 +51,9 @@ type beginTrailOptions struct { repoURL string repoProvider string repoNameExplicit bool - commitExplicit bool - repoRootExplicit bool + // flags lets commitInfoRequest tell a passed --commit/--repo-root from a + // defaulted one. + flags *pflag.FlagSet } type TrailPayload struct { @@ -87,8 +89,7 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { o.repoNameExplicit = cmd.Flags().Changed("repository") - o.commitExplicit = cmd.Flags().Changed("commit") - o.repoRootExplicit = cmd.Flags().Changed("repo-root") + o.flags = cmd.Flags() return o.run(args) }, } @@ -133,11 +134,10 @@ func (o *beginTrailOptions) run(args []string) error { if o.commitSHA != "" { o.payload.Commit, err = commitInfoRequest{ - repoRoot: o.srcRepoRoot, - sha: o.commitSHA, - redacted: o.redactedCommitInfo, - commitExplicit: o.commitExplicit, - repoRootExplicit: o.repoRootExplicit, + repoRoot: o.srcRepoRoot, + sha: o.commitSHA, + redacted: o.redactedCommitInfo, + flags: o.flags, }.resolve() if err != nil { return err