Skip to content
72 changes: 60 additions & 12 deletions cmd/kosli/attestJira.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type attestJiraOptions struct {
projectKeys []string
issueFields string
secondarySource string
trailerKey string
ignoreBranchMatch bool
assert bool
payload JiraAttestationPayload
Expand All @@ -38,8 +39,13 @@ type attestJiraOptions struct {
const attestJiraShortDesc = `Report a jira attestation to an artifact or a trail in a Kosli flow. `

const attestJiraLongDesc = attestJiraShortDesc + `
Parses the given commit's message, current branch name or the content of the ^--jira-secondary-source^
argument for Jira issue references of the form:
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references of the form.
Comment on lines +42 to +43

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover from splitting the sentence: line 43 ends for Jira issue references of the form. — the "of the form" now dangles, since the form is defined two lines later under its own heading. This is the first thing kosli attest jira --help prints.

Suggested change
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references of the form.
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references.

Use ^--jira-trailer^ to read issue keys exclusively from a named git trailer line instead
(e.g. ^Jira: PROJ-42^); when set, the commit message body, branch name, and
^--jira-secondary-source^ are not scanned.
Comment on lines +42 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph now contradicts the validation added in this same commit. It tells the user that with --jira-trailer set, --jira-secondary-source "is not scanned" — but MuXRequiredFlags(cmd, []string{"jira-trailer", "jira-secondary-source"}, false) at line 260 makes passing both a hard error, not a silently-ignored flag. A user who reads this help and writes both gets Error: only one of --jira-trailer, --jira-secondary-source is allowed.

Also a leftover from the last round: line 43 still ends for Jira issue references of the form. — "of the form" dangles now that the form is defined two lines below under its own heading.

Suggested change
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references of the form.
Use ^--jira-trailer^ to read issue keys exclusively from a named git trailer line instead
(e.g. ^Jira: PROJ-42^); when set, the commit message body, branch name, and
^--jira-secondary-source^ are not scanned.
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references.
Use ^--jira-trailer^ to read issue keys exclusively from a named git trailer line instead
(e.g. ^Jira: PROJ-42^); when set, neither the commit message body nor the branch name is
scanned, and ^--jira-secondary-source^ may not be used at the same time.

Comment on lines +42 to +46

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph tells the user something the code now rejects. Line 46 says that when --jira-trailer is set, --jira-secondary-source "is not scanned" — but MuXRequiredFlags(cmd, []string{"jira-trailer", "jira-secondary-source"}, false) at line 260 (added in this same PR, and pinned by test 30) makes passing both a hard error. A user who reads this help and passes both gets Error: only one of --jira-trailer, --jira-secondary-source is allowed, not a quietly-ignored flag.

Also still open from the previous round: line 43 ends for Jira issue references of the form. — "of the form" dangles, since the form is now defined two lines down under its own heading. This is the first thing kosli attest jira --help prints.

Suggested change
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references of the form.
Use ^--jira-trailer^ to read issue keys exclusively from a named git trailer line instead
(e.g. ^Jira: PROJ-42^); when set, the commit message body, branch name, and
^--jira-secondary-source^ are not scanned.
By default, parses the given commit's message, current branch name, or the content of the
^--jira-secondary-source^ argument for Jira issue references.
Use ^--jira-trailer^ to read issue keys exclusively from a named git trailer line instead
(e.g. ^Jira: PROJ-42^); when set, neither the commit message body nor the branch name is
scanned, and ^--jira-secondary-source^ may not be used at the same time.

Fix this →


Jira issue references have the form:
'at least 2 characters long, starting with an uppercase letter project key followed by
dash and one or more digits'.

Expand All @@ -59,13 +65,16 @@ because ^CVE-2026^ would be followed by ^-4^. This applies across all parsed sou
(commit message, branch name, and secondary source).
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-secondary-source^ with a different identifier
format as a workaround.
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which bypasses pattern-scanning entirely.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.
Comment on lines 66 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"bypasses pattern-scanning entirely" isn't true, and this paragraph is the one place a user acts on it.

Trailer values still go through jira.FindJiraIssueKeys (attestJira.go:344), which applies both the key regex and isPartialMultiSegment. --jira-trailer changes which text is scanned, not how it is parsed.

Concretely, a user with project key CVE who reads this and writes Jira: CVE-2026-41284 gets the regex match CVE-2026, whose only occurrence is followed by -4, so it is filtered out and the attestation is non-compliant — the exact outcome the paragraph promises to avoid. (The new logger.Warn at line 347 does at least surface it now, which is a good addition.)

The flag genuinely does help — it removes the surrounding commit text that causes most collisions — so the fix is just to scope the claim:

Suggested change
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-secondary-source^ with a different identifier
format as a workaround.
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which bypasses pattern-scanning entirely.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which narrows the scanned text to the trailer value
so unrelated identifiers elsewhere in the commit cannot interfere. The same pattern rules
still apply to the trailer value itself.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.

Fix this →

Comment on lines 66 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"bypasses pattern-scanning entirely" is still not true — this was raised last round and the wording is unchanged.

Trailer values are joined and passed straight to jira.FindJiraIssueKeys at line 345, so both the key regex and the isPartialMultiSegment filter described in the paragraph above still apply. --jira-trailer changes which text is scanned, not how it is parsed.

This matters here specifically because this is the CVE-collision paragraph: a user with project key CVE who follows this advice and writes Jira: CVE-2026-41284 gets CVE-2026 matched and then filtered out (followed by -4), producing a non-compliant attestation — the exact outcome the paragraph promises the flag avoids. The new logger.Warn at line 348 does at least surface it now, which is a good addition.

The flag genuinely does help — it removes the surrounding commit text that causes most collisions — so it's just the scope of the claim that needs fixing:

Suggested change
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-secondary-source^ with a different identifier
format as a workaround.
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which bypasses pattern-scanning entirely.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which narrows the scanned text to the trailer value
so unrelated identifiers elsewhere in the commit cannot interferenote the same pattern
rules still apply to the trailer value itself.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.

Fix this →

Comment on lines 66 to +70

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"bypasses pattern-scanning entirely" is still inaccurate — third round on this one, wording unchanged.

Trailer values are joined and handed to jira.FindJiraIssueKeys at line 345, exactly like the scanning path, so both the key regex and the isPartialMultiSegment filter described in the paragraph directly above still apply. --jira-trailer changes which text is scanned, not how it is parsed.

It matters here specifically because this is the CVE-collision paragraph — the one place the user acts on this claim. A user with project key CVE who follows the advice and writes Jira: CVE-2026-41284 gets CVE-2026 matched and then filtered out (its only occurrence is followed by -4), producing a non-compliant attestation: the exact outcome the paragraph promises the flag avoids. The new logger.Warn at line 348 does now surface it, which is a genuine improvement, but the prose still points the user at a workaround that doesn't work for this case.

The flag does help — it removes the surrounding commit text that causes most collisions — so only the scope of the claim needs fixing:

Suggested change
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-secondary-source^ with a different identifier
format as a workaround.
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which bypasses pattern-scanning entirely.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.
Note: if your Jira project key collides with this pattern (e.g. a project key of ^CVE^), an
issue reference that happens to be the prefix of a longer hyphenated number (such as a CVE
identifier) will be filtered out. Use ^--jira-trailer^ to read issue keys from a dedicated
git trailer line (e.g. ^Jira: CVE-42^), which narrows the scanned text to the trailer value
so unrelated identifiers elsewhere in the commit cannot interferenote the same pattern
rules still apply to the trailer value itself.
Alternatively, use ^--jira-secondary-source^ with a different identifier format.

Fix this →


If you want to restrict the Jira issue matching to a specific project, use the
^--jira-project-key^ flag to specify your own project key. You can specify multiple project keys if needed.

If the ^--ignore-branch-match^ is set, the branch name is not parsed for a match.
^--ignore-branch-match^ has no effect when ^--jira-trailer^ is set, since the branch is
never scanned in trailer mode.

The found issue references will be checked against Jira to confirm their existence.
The attestation is reported in all cases, and its compliance status depends on referencing
Expand Down Expand Up @@ -190,6 +199,20 @@ kosli attest jira \
--jira-api-token yourJiraAPIToken \
--api-token yourAPIToken \
--org yourOrgName

# read the jira issue key exclusively from a git trailer line (e.g. "Jira: PROJ-42")
# bypasses commit message and branch scanning entirely — useful when project keys
# collide with patterns like CVE identifiers
kosli attest jira \
--name yourAttestationName \
--flow yourFlowName \
--trail yourTrailName \
--jira-trailer Jira \
--jira-base-url https://kosli.atlassian.net \
--jira-username user@domain.com \
--jira-api-token yourJiraAPIToken \
--api-token yourAPIToken \
--org yourOrgName
`

func newAttestJiraCmd(out io.Writer) *cobra.Command {
Expand Down Expand Up @@ -234,6 +257,11 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command {
return err
}

err = MuXRequiredFlags(cmd, []string{"jira-trailer", "jira-secondary-source"}, false)
if err != nil {
return err
}

err = ValidateSliceValues(o.redactedCommitInfo, allowedCommitRedactionValues)
if err != nil {
return fmt.Errorf("%s for --redact-commit-info", err.Error())
Expand Down Expand Up @@ -263,6 +291,7 @@ func newAttestJiraCmd(out io.Writer) *cobra.Command {
cmd.Flags().StringSliceVar(&o.projectKeys, "jira-project-key", []string{}, jiraProjectKeyFlag)
cmd.Flags().StringVar(&o.issueFields, "jira-issue-fields", "", jiraIssueFieldFlag)
cmd.Flags().StringVar(&o.secondarySource, "jira-secondary-source", "", jiraSecondarySourceFlag)
cmd.Flags().StringVar(&o.trailerKey, "jira-trailer", "", jiraTrailerFlag)
cmd.Flags().BoolVar(&o.ignoreBranchMatch, "ignore-branch-match", false, ignoreBranchMatchFlag)
cmd.Flags().BoolVar(&o.assert, "assert", false, attestationAssertFlag)

Expand Down Expand Up @@ -304,11 +333,30 @@ func (o *attestJiraOptions) run(args []string) error {
return err
}

// Search commit message, branch name, and secondary source for Jira issue keys,
// filtering out false positives from multi-segment identifiers like CVE-2026-41284.
issueIDs := jira.FindJiraIssueKeys(jiraSearchText(commitInfo, o.secondarySource, o.ignoreBranchMatch), o.projectKeys)
logger.Debug("Checked for Jira issue references in Git commit %s on branch %s commit message:\n%s", commitInfo.Sha1, commitInfo.Branch, commitInfo.Message)
logger.Debug("the following Jira references are found in commit message or branch name: %v", issueIDs)
// Find Jira issue keys either from a named git trailer or by scanning the
// commit message, branch name, and secondary source.
var issueIDs []string
if o.trailerKey != "" {
if o.ignoreBranchMatch {
logger.Warn("--ignore-branch-match has no effect when --jira-trailer is set")
}
trailerValues := gitview.GetTrailerValues(commitInfo.Message, o.trailerKey)
combinedTrailerText := strings.Join(trailerValues, "\n")
issueIDs = jira.FindJiraIssueKeys(combinedTrailerText, o.projectKeys)
logger.Debug("Checked for Jira issue references in trailer '%s' of Git commit %s: %v", o.trailerKey, commitInfo.Sha1, trailerValues)
Comment thread
vidhu-balad marked this conversation as resolved.
if len(trailerValues) > 0 && len(issueIDs) == 0 {
logger.Warn("trailer '%s' was found but contained no valid Jira issue keys: %v", o.trailerKey, trailerValues)
}
} else {
issueIDs = jira.FindJiraIssueKeys(jiraSearchText(commitInfo, o.secondarySource, o.ignoreBranchMatch), o.projectKeys)
logger.Debug("Checked for Jira issue references in Git commit %s on branch %s commit message:\n%s", commitInfo.Sha1, commitInfo.Branch, commitInfo.Message)
}
logger.Debug("the following Jira references are found: %v", issueIDs)
Comment thread
vidhu-balad marked this conversation as resolved.

issueSource := "commit message or branch name"
if o.trailerKey != "" {
issueSource = fmt.Sprintf("trailer '%s'", o.trailerKey)
}

issueLog := ""
issueFoundCount := 0
Expand Down Expand Up @@ -368,7 +416,7 @@ func (o *attestJiraOptions) run(args []string) error {
if err != nil {
errString = fmt.Sprintf("%s\nError: ", err.Error())
}
err = fmt.Errorf("%sno Jira references are found in commit message or branch name", errString)
err = fmt.Errorf("%sno Jira references are found in %s", errString, issueSource)
}

if issueFoundCount != len(issueIDs) && o.assert && !global.DryRun {
Expand All @@ -381,8 +429,8 @@ func (o *attestJiraOptions) run(args []string) error {
for _, reason := range unconfirmedReasons {
reasonLog += fmt.Sprintf("\n\treason: %s", reason)
}
err = fmt.Errorf("%s%s from references found in commit message or branch name%s%s", errString,
jiraAssertHeadline(len(issueIDs)-issueFoundCount-len(unconfirmedIDs), len(unconfirmedIDs)), issueLog, reasonLog)
err = fmt.Errorf("%s%s from references found in %s%s%s", errString,
jiraAssertHeadline(len(issueIDs)-issueFoundCount-len(unconfirmedIDs), len(unconfirmedIDs)), issueSource, issueLog, reasonLog)
}
return wrapAttestationError(err)
}
Expand Down
56 changes: 56 additions & 0 deletions cmd/kosli/attestJira_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,62 @@ func (suite *AttestJiraCommandTestSuite) TestAttestJiraCmd() {
cmd: fmt.Sprintf("attest jira --name .foo --commit HEAD --jira-base-url https://kosli-test.atlassian.net %s", suite.defaultKosliArguments),
golden: "Error: failed to parse attestation name: invalid attestation name format: .foo\n",
},
{
name: "27 can attest jira using --jira-trailer to extract issue key from commit trailer",
cmd: fmt.Sprintf(`attest jira --name bar
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--assert
--repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments),
golden: "jira attestation 'bar' is reported to trail: test-123\n",
additionalConfig: jiraTestsAdditionalConfig{
commitMessage: "fix: some change\n\nJira: EX-1\nOna-Environment-Id: ONA-999",
},
},
Comment thread
vidhu-balad marked this conversation as resolved.
Comment thread
vidhu-balad marked this conversation as resolved.
{
name: "28 --jira-trailer with no matching trailer produces no issue IDs (non-compliant but reported)",
cmd: fmt.Sprintf(`attest jira --name bar
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments),
golden: "jira attestation 'bar' is reported to trail: test-123\n",
additionalConfig: jiraTestsAdditionalConfig{
commitMessage: "fix: some change with no jira trailer",
},
},
{
wantError: true,
name: "29 --jira-trailer with --assert fails when trailer is absent",
cmd: fmt.Sprintf(`attest jira --name bar
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--assert
--repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments),
golden: "jira attestation 'bar' is reported to trail: test-123\nError: no Jira references are found in trailer 'Jira'\n",
additionalConfig: jiraTestsAdditionalConfig{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three behaviours added in the latest commit have no test, which is worth closing given the TDD discipline in CLAUDE.md — the first in particular is a new error path that changes what a valid command line is:

  1. MuXRequiredFlags(cmd, []string{"jira-trailer", "jira-secondary-source"}, false) (attestJira.go:260) — nothing pins the error. A one-line golden case locks it in and documents the resolution of that thread.
  2. The branch-name half of the PR's headline claim — test 27 proves the commit body isn't scanned (ONA-999 would break the assert); nothing proves the branch isn't. execJiraTestCase already supports branchName (line 414).
  3. The two new logger.Warn branches (attestJira.go:338 and 347) — optional, but (2) below is nearly free.
Suggested change
additionalConfig: jiraTestsAdditionalConfig{
{
wantError: true,
name: "30 --jira-trailer and --jira-secondary-source are mutually exclusive",
cmd: fmt.Sprintf(`attest jira --name bar --commit HEAD
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--jira-secondary-source some-branch %s`, suite.defaultKosliArguments),
golden: "Error: only one of --jira-trailer, --jira-secondary-source is allowed\n",
},
{
name: "31 --jira-trailer does not scan the branch name for issue references",
cmd: fmt.Sprintf(`attest jira --name bar
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--assert
--repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments),
golden: "jira attestation 'bar' is reported to trail: test-123\n",
additionalConfig: jiraTestsAdditionalConfig{
branchName: "bugfix/ONA-999",
commitMessage: "fix: some change\n\nJira: EX-1",
},
},
}

ONA-999 in the branch is the right probe for (2): it matches the Jira key pattern, so under the old scanning behaviour it becomes an unresolvable issue ID and issueFoundCount != len(issueIDs) fails the assert.

Fix this →

commitMessage: "fix: some change with no jira trailer",
},
},
{
wantError: true,
name: "30 --jira-trailer and --jira-secondary-source are mutually exclusive",
cmd: fmt.Sprintf("attest jira --name bar --jira-base-url https://kosli-test.atlassian.net --jira-trailer Jira --jira-secondary-source foo --commit HEAD --repo-root %s %s", suite.tmpDir, suite.defaultKosliArguments),
golden: "Error: only one of --jira-trailer, --jira-secondary-source is allowed\n",
},
{
wantError: true,
name: "31 --jira-trailer does not scan branch name even when branch contains a Jira key",
cmd: fmt.Sprintf(`attest jira --name bar
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--assert
--repo-root %s %s`, suite.tmpDir, suite.defaultKosliArguments),
golden: "jira attestation 'bar' is reported to trail: test-123\nError: no Jira references are found in trailer 'Jira'\n",
additionalConfig: jiraTestsAdditionalConfig{
branchName: "EX-1-some-feature",
commitMessage: "fix: some change with no jira trailer",
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test 30 is a good one — EX-1 in the branch resolves in Jira under the old scanning path, so wantError: true genuinely fails if branch scanning leaks into trailer mode. That closes the branch half of the PR's headline claim.

The one remaining gap is the new error path added in this commit: MuXRequiredFlags(cmd, []string{"jira-trailer", "jira-secondary-source"}, false) (attestJira.go:260) changes what a valid command line is, and nothing pins it. Per the TDD discipline in CLAUDE.md, a new error path should arrive with a test — and this one is a one-liner:

Suggested change
},
{
wantError: true,
name: "31 --jira-trailer and --jira-secondary-source are mutually exclusive",
cmd: fmt.Sprintf(`attest jira --name bar --commit HEAD
--jira-base-url https://kosli-test.atlassian.net
--jira-trailer Jira
--jira-secondary-source some-branch %s`, suite.defaultKosliArguments),
golden: "Error: only one of --jira-trailer, --jira-secondary-source is allowed\n",
},
}

Note this case must not carry additionalConfig, since execJiraTestCase appends --commit <sha> only when additionalConfig is non-nil (line 426) — hence the explicit --commit HEAD above.

Fix this →

},
}

for _, test := range tests {
Expand Down
1 change: 1 addition & 0 deletions cmd/kosli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ The ^.kosli_ignore^ will be treated as part of the artifact like any other file,
jiraIssueFieldFlag = "[optional] The comma separated list of fields to include from the Jira issue. Default no fields are included. '*all' will give all fields."
jiraSecondarySourceFlag = "[optional] An optional string to search for Jira ticket reference, e.g. '--jira-secondary-source ${{ github.head_ref }}'"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jiraTrailerFlag now carries "Mutually exclusive with --jira-secondary-source." — thanks, that closes the main half of the earlier thread. The reciprocal sentence on jiraSecondarySourceFlag is still missing, so a user reading --jira-secondary-source's help alone has no way to discover the constraint.

It's worth the one line because bindFlags sets flags from env/config via cmd.Flags().Set(...) (root.go:673), which marks them Changed. An org that exports KOSLI_JIRA_SECONDARY_SOURCE in a shared CI step and then adds --jira-trailer Jira on one job's command line gets a hard error from a flag they never typed.

Suggested change
jiraSecondarySourceFlag = "[optional] An optional string to search for Jira ticket reference, e.g. '--jira-secondary-source ${{ github.head_ref }}'"
jiraSecondarySourceFlag = "[optional] An optional string to search for Jira ticket reference, e.g. '--jira-secondary-source ${{ github.head_ref }}'. Mutually exclusive with --jira-trailer."

ignoreBranchMatchFlag = "Ignore branch name when searching for Jira ticket reference."
jiraTrailerFlag = "[optional] The git trailer key to use as the sole source of Jira issue references (e.g. '--jira-trailer Jira' extracts the value of 'Jira: <issue-key>' lines from the commit message). When set, the commit message body and branch name are not scanned. Mutually exclusive with --jira-secondary-source."
envDescriptionFlag = "[optional] The environment description."
flowDescriptionFlag = "[optional] The Kosli flow description."
trailDescriptionFlag = "[optional] The Kosli trail description."
Expand Down
1 change: 1 addition & 0 deletions cmd/kosli/testdata/empty-flag-audit-coverage.json
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@
"jira-pat": "string",
"jira-project-key": "stringSlice",
"jira-secondary-source": "string",
"jira-trailer": "string",
"jira-username": "string",
"name": "string",
"origin-url": "string",
Expand Down
18 changes: 18 additions & 0 deletions internal/gitview/gitView.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,24 @@ func getCommitURL(repoURL, commitHash string) string {
}
}

// GetTrailerValues extracts the values of all trailer lines in a commit message
// that match the given key. The key comparison is case-insensitive. Trailer lines
// have the format "<key>: <value>". Returns an empty (non-nil) slice if none are found.
Comment thread
vidhu-balad marked this conversation as resolved.
Comment on lines +279 to +281

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key trimming is in and unit-tested (TrimSpace + TrimRight(":"), with cases at gitView_test.go:541 and :547) — that closes the last round's behavioural point. The doc comment didn't move with it, and one of the gaps has a user-visible consequence:

  1. Empty values are silently dropped (line 289) — a sensible choice, but undocumented and the only branch of this function with no unit case. It also quietly defeats the new warning in attestJira.go:347: a commit with a bare Jira: line yields len(trailerValues) == 0, so the user gets neither the issue key nor the "trailer found but no valid Jira issue keys" warning — and that's precisely the case where they demonstrably tried to use the trailer and got it wrong. Worth a unit case pinning the drop either way.
  2. The name promises git-trailer semantics the implementation doesn't have — real trailers (per git interpret-trailers) live only in the message's final paragraph; this matches <key>: on any line, including the subject. Harmless for this caller since values go through the Jira key regex anyway, but the comment should say so rather than leave the next caller to assume otherwise.
Suggested change
// GetTrailerValues extracts the values of all trailer lines in a commit message
// that match the given key. The key comparison is case-insensitive. Trailer lines
// have the format "<key>: <value>". Returns an empty (non-nil) slice if none are found.
// GetTrailerValues returns the values of every line in a commit message of the form
// "<key>: <value>". The key comparison is case-insensitive; surrounding whitespace on
// both the key and the line is ignored, and a trailing ":" on the key is tolerated.
// Note this matches any such line anywhere in the message, not only trailers in the
// final paragraph as `git interpret-trailers` defines them. Lines with an empty value
// are skipped. Returns an empty (non-nil) slice if none are found.

Fix this →

func GetTrailerValues(message, key string) []string {
Comment on lines +279 to +282

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key trimming from last round is in (TrimSpace + TrimRight(":"), both with unit cases — nice). Two doc items are still open, and one has a real behavioural consequence:

  1. Empty values are silently dropped (line 290) — a sensible choice, but undocumented and untested. It also quietly defeats the new warning in attestJira.go:347: a commit with a bare Jira: line yields len(trailerValues) == 0, so the user gets neither the issue key nor the "trailer found but no valid keys" warning — the one case where the user demonstrably tried to use the trailer and typoed it.
  2. The name promises git-trailer semantics the implementation doesn't have — real trailers (per git interpret-trailers) live only in the message's last paragraph; this matches <key>: on any line, including the subject. Harmless for this caller, but the comment should say so rather than the next caller assuming otherwise.
Suggested change
// GetTrailerValues extracts the values of all trailer lines in a commit message
// that match the given key. The key comparison is case-insensitive. Trailer lines
// have the format "<key>: <value>". Returns an empty (non-nil) slice if none are found.
func GetTrailerValues(message, key string) []string {
// GetTrailerValues returns the values of every line in a commit message of the form
// "<key>: <value>". The key comparison is case-insensitive; surrounding whitespace on
// both the key and the line is ignored, and a trailing ":" on the key is tolerated.
// Note this matches any such line anywhere in the message, not only trailers in the
// final paragraph as `git interpret-trailers` defines them. Lines with an empty value
// are skipped. Returns an empty (non-nil) slice if none are found.

result := []string{}
prefix := strings.ToLower(strings.TrimRight(strings.TrimSpace(key), ":")) + ":"
for _, line := range strings.Split(message, "\n") {
trimmed := strings.TrimSpace(line)
if strings.HasPrefix(strings.ToLower(trimmed), prefix) {
value := strings.TrimSpace(trimmed[len(prefix):])
if value != "" {
result = append(result, value)
}
}
}
return result
}

// ResolveRevision returns an explicit commit SHA1 from commit SHA or ref (e.g. HEAD~2)
func (gv *GitView) ResolveRevision(commitSHAOrRef string) (string, error) {
hash, err := gv.repository.ResolveRevision(plumbing.Revision(commitSHAOrRef))
Expand Down
69 changes: 69 additions & 0 deletions internal/gitview/gitView_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,75 @@ func initializeRepoAndCommit(repoPath string, commitsNumber int) (*git.Repositor
return repo, w, nil
}

func (suite *GitViewTestSuite) TestGetTrailerValues() {
for _, tt := range []struct {
name string
message string
key string
expected []string
}{
{
name: "no trailers returns empty slice",
message: "fix: something\n\nsome body text",
key: "Jira",
expected: []string{},
},
{
name: "single matching trailer",
message: "fix: something\n\nJira: BX-123",
key: "Jira",
expected: []string{"BX-123"},
},
{
name: "key match is case-insensitive",
message: "fix: something\n\njira: BX-123",
key: "Jira",
expected: []string{"BX-123"},
},
{
name: "multiple occurrences of same key",
message: "fix: something\n\nJira: BX-123\nJira: BX-456",
key: "Jira",
expected: []string{"BX-123", "BX-456"},
},
{
name: "non-matching trailers are ignored",
message: "fix: something\n\nJira: BX-123\nOna-Environment-Id: ONA-456",
key: "Jira",
expected: []string{"BX-123"},
},
{
name: "whitespace trimmed from value",
message: "fix: something\n\nJira: BX-123 ",
key: "Jira",
expected: []string{"BX-123"},
},
{
name: "leading whitespace on line is tolerated",
message: "fix: something\n\n Jira: BX-123",
key: "Jira",
expected: []string{"BX-123"},
},
{
name: "key supplied with trailing colon still matches",
message: "fix: something\n\nJira: BX-123",
key: "Jira:",
expected: []string{"BX-123"},
},
{
name: "key with surrounding whitespace still matches",
message: "fix: something\n\nJira: BX-123",
key: " Jira ",
expected: []string{"BX-123"},
},
} {
suite.Run(tt.name, func() {
result := GetTrailerValues(tt.message, tt.key)
require.Equal(suite.T(), tt.expected, result)
})
}
}

func TestGitViewTestSuite(t *testing.T) {
suite.Run(t, new(GitViewTestSuite))
}
Loading