diff --git a/action.yml b/action.yml index 58cca79..5c50075 100644 --- a/action.yml +++ b/action.yml @@ -48,6 +48,9 @@ inputs: issues-token: description: GitHub App installation token for `issues-repository` only, with the "Issues" and organisation "Projects" permissions in read and write. tamis refuses it when it also reaches `repository`, so a bug cannot publish a report there. required: false + close-duplicates: + description: Close the advisory of an issue a maintainer closed as a duplicate, credit its reporter on the original report and tell them on the closed advisory. Needs `issues-repository`. + default: "false" comment: description: Post the triage as a comment on the advisory. Off by default, every triage then only reaches `project`, which must be set. default: "false" @@ -143,6 +146,7 @@ runs: PROJECT: ${{ inputs.project }} ISSUES_REPOSITORY: ${{ inputs.issues-repository }} ISSUES_TOKEN: ${{ inputs.issues-token }} + CLOSE_DUPLICATES: ${{ inputs.close-duplicates }} COMMENT: ${{ inputs.comment }} PACKS: ${{ inputs.packs }} - name: Store private reports diff --git a/cmd/tamis/main.go b/cmd/tamis/main.go index a48556f..19acb58 100644 --- a/cmd/tamis/main.go +++ b/cmd/tamis/main.go @@ -205,6 +205,15 @@ func run(ctx context.Context, log *slog.Logger) error { } cfg.Issues = github.New(issues) } + if v := os.Getenv("CLOSE_DUPLICATES"); v != "" { + if cfg.CloseDuplicates, err = strconv.ParseBool(v); err != nil { + return fmt.Errorf("CLOSE_DUPLICATES: %w", err) + } + } + // Only an issue carries the decision of a maintainer, a draft item has no close reason. + if cfg.CloseDuplicates && cfg.IssuesRepository == "" { + return errors.New("CLOSE_DUPLICATES needs ISSUES_REPOSITORY") + } eng, agent, model, err := newEngine(os.Getenv("ENGINE"), os.Getenv("MODEL"), os.Getenv("LLM_API_KEY")) if err != nil { return err diff --git a/cmd/tamis/main_test.go b/cmd/tamis/main_test.go index fee3d24..0c65f7d 100644 --- a/cmd/tamis/main_test.go +++ b/cmd/tamis/main_test.go @@ -71,11 +71,13 @@ func TestRunValidatesConfiguration(t *testing.T) { "issues token required": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "ISSUES_REPOSITORY": "o/r-security"}, "ISSUES_REPOSITORY needs an ISSUES_TOKEN of its own"}, "issues token reused": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "ISSUES_REPOSITORY": "o/r-security", "ISSUES_TOKEN": "t"}, "ISSUES_REPOSITORY needs an ISSUES_TOKEN of its own"}, "invalid force": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "FORCE": "maybe"}, "FORCE"}, + "invalid close": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "CLOSE_DUPLICATES": "maybe"}, "CLOSE_DUPLICATES"}, + "close without issues": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "CLOSE_DUPLICATES": "true"}, "CLOSE_DUPLICATES needs ISSUES_REPOSITORY"}, "invalid comment": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "COMMENT": "maybe"}, "COMMENT"}, "references required": {map[string]string{"REPOSITORY": "o/r", "SINCE": "2026-10-22", "GH_TOKEN": "t", "LLM_API_KEY": "k", "OUTPUT_DIR": t.TempDir()}, "REFERENCES must list"}, } { t.Run(name, func(t *testing.T) { - for _, k := range []string{"REPOSITORY", "GITHUB_REPOSITORY", "ADVISORY", "REPO_PATH", "BOT_LOGIN", "DAYBREAK", "SINCE", "GH_TOKEN", "ENGINE", "MODEL", "LLM_API_KEY", "OUTPUT_DIR", "REFERENCES", "PROJECT", "ISSUES_REPOSITORY", "ISSUES_TOKEN", "COMMENT", "PACKS", "FORCE"} { + for _, k := range []string{"REPOSITORY", "GITHUB_REPOSITORY", "ADVISORY", "REPO_PATH", "BOT_LOGIN", "DAYBREAK", "SINCE", "GH_TOKEN", "ENGINE", "MODEL", "LLM_API_KEY", "OUTPUT_DIR", "REFERENCES", "PROJECT", "ISSUES_REPOSITORY", "ISSUES_TOKEN", "CLOSE_DUPLICATES", "COMMENT", "PACKS", "FORCE"} { t.Setenv(k, tc.env[k]) } diff --git a/docs/configuration.md b/docs/configuration.md index a2a55e2..a4efd0b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -20,11 +20,12 @@ | `project` | | Private organisation project kept in sync with the advisories, as `org/number`. See [the project board](setup.md#5-project-board). | | `issues-repository` | | Private repository the board items are created in as issues, so duplicates become sub-issues of their original report. Drafts when unset. See [the project board](setup.md#5-project-board). | | `issues-token` | | Installation token for `issues-repository` only, with the "Issues" and organisation "Projects" permissions in read and write. Required with `issues-repository`. | +| `close-duplicates` | `false` | Closes the advisory of an issue a maintainer closed as a duplicate. Needs `issues-repository`. See [the project board](setup.md#5-project-board). | | `runner-image` | tamis agent image, pinned by digest | Image the agent runs in. It must contain the agent CLI, git, curl and `harness-proxy`. The action skips the attestation check for a custom image. | The action refuses to run from a public workflow repository or from a `uses:` that is not a full commit SHA. Update that SHA through a reviewed pull request. -Outside GitHub Actions, tamis reads the same settings from environment variables: `LLM_API_KEY`, `GH_TOKEN`, `BOT_LOGIN`, `REPOSITORY`, `REPO_PATH`, `SINCE`, `ADVISORY`, `ENGINE`, `MODEL`, `DAYBREAK`, `RUNNER_IMAGE`, `REFERENCES`, `PROJECT`, `ISSUES_REPOSITORY`, `ISSUES_TOKEN`, `COMMENT` and `PACKS`. `OUTPUT_DIR` must name a private directory for the reports. The action creates it in the runner's temporary storage and uploads it to the workflow repository with a seven-day retention. +Outside GitHub Actions, tamis reads the same settings from environment variables: `LLM_API_KEY`, `GH_TOKEN`, `BOT_LOGIN`, `REPOSITORY`, `REPO_PATH`, `SINCE`, `ADVISORY`, `ENGINE`, `MODEL`, `DAYBREAK`, `RUNNER_IMAGE`, `REFERENCES`, `PROJECT`, `ISSUES_REPOSITORY`, `ISSUES_TOKEN`, `CLOSE_DUPLICATES`, `COMMENT` and `PACKS`. `OUTPUT_DIR` must name a private directory for the reports. The action creates it in the runner's temporary storage and uploads it to the workflow repository with a seven-day retention. A local run alone also reads `FORCE=true`, which triages every selected advisory again, `ADVISORY` alone or every one in triage, and updates its board item even when it already holds a triage. It needs `COMMENT` off, and a failed new triage leaves the previous one on the board. The action has no such input. diff --git a/docs/setup.md b/docs/setup.md index 1f3486c..4912cb1 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -136,4 +136,6 @@ tamis writes the issues and adds them to the project with `issues-token`, a seco Then pass `issues-token: ${{ steps.issues.outputs.token }}`. The App needs the "Issues" permission in read and write and an installation on the issues repository. -With `comment` left at `false`, tamis posts nothing on the advisories. The item of an advisory says it was triaged, `Attempts` counts its failed attempts in place of the failure comments, and deleting an item triages its advisory again on the next run. +With `close-duplicates: true`, closing an issue as a duplicate decides its report. At the start of the next run, tamis credits the reporter as `reporter` on the `Original report` of the issue when that report is still in `triage` or `draft`, comments on the advisory to name that report, and closes it. A note on the issue then names the original and the login credited on it. The original is always the one of `Original report`, whichever issue the duplicate was closed against, and an advisory without one stays as it is. The comment names the original even under embargo, and whoever can close an issue of that repository can then close an advisory. + +With `comment` left at `false`, tamis posts nothing on the advisories but the comment of `close-duplicates`. The item of an advisory says it was triaged, `Attempts` counts its failed attempts in place of the failure comments, and deleting an item triages its advisory again on the next run. diff --git a/internal/board/board.go b/internal/board/board.go index e00b474..c6f4f1d 100644 --- a/internal/board/board.go +++ b/internal/board/board.go @@ -174,6 +174,8 @@ type item struct { // issue, zero without one. issue int issueID, parent int64 + // closedAs is the state reason of a closed issue, such as "duplicate", empty while it is open. + closedAs string // triage is nil for an item without one, whose priority is a status Sync leaves alone. triage *Triage } @@ -330,7 +332,16 @@ func Open(ctx context.Context, gh GitHub, ig Issues, project, issues, advisories return nil, err } b.unlisted = map[string]github.Issue{} + byIssue := map[int]*item{} + for _, it := range b.items { + if it.issue != 0 { + byIssue[it.issue] = it + } + } for _, i := range all { + if it := byIssue[i.Number]; it != nil && i.State == "closed" { + it.closedAs = i.StateReason + } if ghsa := strings.ToLower(markedAdvisory(i.Body)); ghsa != "" && b.items[ghsa] == nil { b.unlisted[ghsa] = i } @@ -641,6 +652,86 @@ func (b *Board) option(field, value string) github.FieldValue { return v } +// Advisories writes the advisories, with the token of their repository. +type Advisories interface { + GetAdvisory(ctx context.Context, repo, ghsa string) (github.Advisory, error) + UpdateAdvisory(ctx context.Context, repo, ghsa string, fields map[string]any) error + ListComments(ctx context.Context, repo, ghsa string) ([]github.Comment, error) + PostComment(ctx context.Context, repo, ghsa, body string) error +} + +// CloseDuplicates closes the advisory of each issue a maintainer closed as a duplicate while that advisory waits +// for a decision, and tells its reporter on it. The reporter is credited on the original report of the group when +// that one waits too. An advisory the board names no original for stays as it is. Each advisory it closes is +// marked closed in advisories, so the rest of the run sees it. +func (b *Board) CloseDuplicates(ctx context.Context, gh Advisories, repo string, advisories []github.Advisory) error { + var errs []error + for i, a := range advisories { + it := b.items[strings.ToLower(a.GHSAID)] + if it == nil || it.closedAs != "duplicate" || it.duplicateOf == "" || a.State != "triage" && a.State != "draft" { + continue + } + // A Duplicate of chain edited by hand into a loop has no oldest report, the one it stops at names another. + original := b.oldest(it.duplicateOf) + if o := b.items[strings.ToLower(original)]; o != nil && o.duplicateOf != "" { + continue + } + credited, err := closeDuplicate(ctx, gh, repo, a, original) + if err != nil { + errs = append(errs, fmt.Errorf("closing %s as a duplicate of %s: %w", a.GHSAID, original, err)) + continue + } + advisories[i].State = "closed" + // The note comes after the close, so a close that fails again next run never posts it twice. + note := fmt.Sprintf("Closed %s as a duplicate of %s", a.GHSAID, original) + if o := b.items[strings.ToLower(original)]; o != nil && o.issue != 0 { + note += fmt.Sprintf(" (#%d)", o.issue) + } + // A login in backticks notifies nobody. + if credited != "" { + note += fmt.Sprintf(", where `%s` is credited as a reporter", credited) + } + if err := b.ig.CommentIssue(ctx, b.issues, it.issue, note+"."); err != nil { + errs = append(errs, fmt.Errorf("noting the close of %s on its issue: %w", a.GHSAID, err)) + } + } + + return errors.Join(errs...) +} + +// closeDuplicate comments before closing, since the next run leaves a closed advisory alone. A close that failed +// after its comment finds that comment on the next run instead of posting it again. It returns the login +// credited on the original, empty when the original credits nobody for it. +func closeDuplicate(ctx context.Context, gh Advisories, repo string, a github.Advisory, original string) (string, error) { + o, err := gh.GetAdvisory(ctx, repo, original) + if err != nil { + return "", err + } + comment := fmt.Sprintf("This report is closed as a duplicate of %s, reported earlier.", o.GHSAID) + credited := "" + if login := a.Author.Login; login != "" && !strings.EqualFold(login, o.Author.Login) && (o.State == "triage" || o.State == "draft") { + if !slices.ContainsFunc(o.Credits, func(c github.Credit) bool { return strings.EqualFold(c.Login, login) }) { + credits := append(o.Credits, github.Credit{Login: login, Type: "reporter"}) + if err := gh.UpdateAdvisory(ctx, repo, o.GHSAID, map[string]any{"credits": credits}); err != nil { + return "", err + } + } + comment += " You are credited on it as a reporter." + credited = login + } + comments, err := gh.ListComments(ctx, repo, a.GHSAID) + if err != nil { + return "", err + } + if !slices.ContainsFunc(comments, func(c github.Comment) bool { return c.Body == comment }) { + if err := gh.PostComment(ctx, repo, a.GHSAID, comment); err != nil { + return "", err + } + } + + return credited, gh.UpdateAdvisory(ctx, repo, a.GHSAID, map[string]any{"state": "closed"}) +} + // Sync mirrors the state of every advisory on its item, counts the duplicates of each report, gives every // report of a duplicate group the identifier of its oldest one and ranks every triage again, so a new priority // rule reaches the board without a model run. It only writes the items that changed. @@ -666,7 +757,8 @@ func (b *Board) Sync(ctx context.Context, advisories []github.Advisory) error { for ghsa, it := range b.items { var values []github.FieldValue if state, ok := states[ghsa]; ok && state != it.state { - if it.issue != 0 { + // An issue a maintainer closed as a duplicate keeps that reason once its advisory closes. + if it.issue != 0 && it.closedAs != "duplicate" { if err := b.ig.UpdateIssue(ctx, b.issues, it.issue, issueState(state)); err != nil { errs = append(errs, err) continue diff --git a/internal/board/board_test.go b/internal/board/board_test.go index 56b4044..0899c17 100644 --- a/internal/board/board_test.go +++ b/internal/board/board_test.go @@ -1,7 +1,9 @@ package board import ( + "context" "errors" + "slices" "strings" "testing" "time" @@ -855,3 +857,248 @@ func TestCreateReportsAFailure(t *testing.T) { }) } } + +type advisories struct { + byID map[string]*github.Advisory + comments map[string][]string + updates int + postErr error + closeErr error +} + +func (f *advisories) GetAdvisory(_ context.Context, _, ghsa string) (github.Advisory, error) { + a, ok := f.byID[ghsa] + if !ok { + return github.Advisory{}, github.ErrNotFound + } + + return *a, nil +} + +func (f *advisories) UpdateAdvisory(_ context.Context, _, ghsa string, fields map[string]any) error { + a := f.byID[ghsa] + if credits, ok := fields["credits"]; ok { + a.Credits = credits.([]github.Credit) + } + if state, ok := fields["state"]; ok { + if f.closeErr != nil { + return f.closeErr + } + a.State = state.(string) + } + f.updates++ + + return nil +} + +func (f *advisories) ListComments(_ context.Context, _, ghsa string) ([]github.Comment, error) { + var all []github.Comment + for _, body := range f.comments[ghsa] { + all = append(all, github.Comment{Body: body}) + } + + return all, nil +} + +func (f *advisories) PostComment(_ context.Context, _, ghsa, body string) error { + if f.postErr != nil { + return f.postErr + } + f.comments[ghsa] = append(f.comments[ghsa], body) + + return nil +} + +func (f *advisories) list() []github.Advisory { + var all []github.Advisory + for _, a := range f.byID { + all = append(all, *a) + } + + return all +} + +// closedAsDuplicate puts GHSA-orig and its duplicate GHSA-dup on a board of issues, closes the issue of GHSA-dup +// with reason and opens the board again, as the next run does. +func closedAsDuplicate(t *testing.T, reason string, duplicateOf string) (*boardtest.Project, *Board) { + t.Helper() + p := boardtest.New() + b, err := Open(t.Context(), p, p, "acme/16", "acme/private-issues", "php/php-src") + require.NoError(t, err) + require.NoError(t, b.Put(t.Context(), github.Advisory{GHSAID: "GHSA-orig", Summary: "s", State: "triage", CreatedAt: reported}, LookFirst, &Triage{}, "")) + require.NoError(t, b.Put(t.Context(), github.Advisory{GHSAID: "GHSA-dup", Summary: "s", State: "triage", CreatedAt: reported.AddDate(0, 0, 1)}, Duplicate, &Triage{DuplicateOf: duplicateOf}, "")) + issue := p.IssueOf(p.Find("GHSA-dup")) + issue.State, issue.StateReason = "closed", reason + b, err = Open(t.Context(), p, p, "acme/16", "acme/private-issues", "php/php-src") + require.NoError(t, err) + + return p, b +} + +func newAdvisories(original github.Advisory) *advisories { + dup := github.Advisory{GHSAID: "GHSA-dup", State: "triage"} + dup.Author.Login = "bob" + + return &advisories{byID: map[string]*github.Advisory{"GHSA-orig": &original, "GHSA-dup": &dup}, comments: map[string][]string{}} +} + +func TestCloseDuplicatesClosesTheReportAndCreditsItsReporterOnTheOriginal(t *testing.T) { + carol := github.Credit{Login: "carol", Type: "analyst"} + for name, tc := range map[string]struct { + state, author string + credits []github.Credit + want []github.Credit + comment, note string + }{ + "original in triage": {"triage", "alice", []github.Credit{carol}, []github.Credit{carol, {Login: "bob", Type: "reporter"}}, "This report is closed as a duplicate of GHSA-orig, reported earlier. You are credited on it as a reporter.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1), where `bob` is credited as a reporter."}, + "original in draft": {"draft", "alice", nil, []github.Credit{{Login: "bob", Type: "reporter"}}, "This report is closed as a duplicate of GHSA-orig, reported earlier. You are credited on it as a reporter.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1), where `bob` is credited as a reporter."}, + "already credited": {"triage", "alice", []github.Credit{{Login: "Bob", Type: "finder"}}, []github.Credit{{Login: "Bob", Type: "finder"}}, "This report is closed as a duplicate of GHSA-orig, reported earlier. You are credited on it as a reporter.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1), where `bob` is credited as a reporter."}, + "same reporter": {"triage", "bob", nil, nil, "This report is closed as a duplicate of GHSA-orig, reported earlier.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1)."}, + "original published": {"published", "alice", nil, nil, "This report is closed as a duplicate of GHSA-orig, reported earlier.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1)."}, + "original closed": {"closed", "alice", nil, nil, "This report is closed as a duplicate of GHSA-orig, reported earlier.", "Closed GHSA-dup as a duplicate of GHSA-orig (#1)."}, + } { + t.Run(name, func(t *testing.T) { + p, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + original := github.Advisory{GHSAID: "GHSA-orig", State: tc.state, Credits: tc.credits} + original.Author.Login = tc.author + f := newAdvisories(original) + + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + + require.Equal(t, tc.want, f.byID["GHSA-orig"].Credits) + require.Equal(t, tc.state, f.byID["GHSA-orig"].State, "the original changed state") + require.Equal(t, "closed", f.byID["GHSA-dup"].State) + require.Equal(t, []string{tc.comment}, f.comments["GHSA-dup"]) + require.Empty(t, f.comments["GHSA-orig"]) + require.Equal(t, []string{tc.note}, p.IssueOf(p.Find("GHSA-dup")).Comments) + require.Empty(t, p.IssueOf(p.Find("GHSA-orig")).Comments) + }) + } +} + +func TestCloseDuplicatesCreditsTheOldestReportOfTheChain(t *testing.T) { + p, b := closedAsDuplicate(t, "duplicate", "GHSA-mid") + require.NoError(t, b.Put(t.Context(), github.Advisory{GHSAID: "GHSA-mid", Summary: "s", State: "triage", CreatedAt: reported}, Duplicate, &Triage{DuplicateOf: "GHSA-orig"}, "")) + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + f.byID["GHSA-mid"] = &github.Advisory{GHSAID: "GHSA-mid", State: "triage"} + + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + + require.Equal(t, []github.Credit{{Login: "bob", Type: "reporter"}}, f.byID["GHSA-orig"].Credits) + require.Empty(t, f.byID["GHSA-mid"].Credits, "the reporter was credited on an intermediate duplicate") + require.Equal(t, "closed", f.byID["GHSA-dup"].State) + require.Equal(t, "triage", f.byID["GHSA-mid"].State) + require.Equal(t, "open", p.IssueOf(p.Find("GHSA-mid")).State) +} + +func TestCloseDuplicatesLeavesTheOtherReportsAlone(t *testing.T) { + for name, tc := range map[string]struct { + reason, duplicateOf, state string + }{ + "issue open": {"", "GHSA-orig", "triage"}, + "issue closed otherwise": {"not_planned", "GHSA-orig", "triage"}, + "no original": {"duplicate", "", "triage"}, + "original is itself": {"duplicate", "GHSA-dup", "triage"}, + "advisory decided": {"duplicate", "GHSA-orig", "published"}, + } { + t.Run(name, func(t *testing.T) { + p, b := closedAsDuplicate(t, tc.reason, tc.duplicateOf) + if tc.reason == "" { + p.IssueOf(p.Find("GHSA-dup")).State = "open" + var err error + b, err = Open(t.Context(), p, p, "acme/16", "acme/private-issues", "php/php-src") + require.NoError(t, err) + } + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + f.byID["GHSA-dup"].State = tc.state + + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + + require.Zero(t, f.updates) + require.Empty(t, f.comments) + }) + } +} + +func TestCloseDuplicatesKeepsTheReportOpenUntilItsCommentIsPosted(t *testing.T) { + _, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + f.postErr = errors.New("HTTP 502") + + require.Error(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + require.Equal(t, "triage", f.byID["GHSA-dup"].State, "a report was closed without telling its reporter") + + f.postErr = nil + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + require.Equal(t, "closed", f.byID["GHSA-dup"].State) + require.Equal(t, []github.Credit{{Login: "bob", Type: "reporter"}}, f.byID["GHSA-orig"].Credits, "the reporter was credited twice") + require.Len(t, f.comments["GHSA-dup"], 1) +} + +func TestCloseDuplicatesPostsItsCommentOnceWhenTheCloseFails(t *testing.T) { + _, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + f.closeErr = errors.New("HTTP 502") + + dup := func(list []github.Advisory) github.Advisory { + return list[slices.IndexFunc(list, func(a github.Advisory) bool { return a.GHSAID == "GHSA-dup" })] + } + for range 2 { + list := f.list() + require.Error(t, b.CloseDuplicates(t.Context(), f, "php/php-src", list)) + require.Equal(t, "triage", f.byID["GHSA-dup"].State) + require.Equal(t, "triage", dup(list).State, "a report that stayed open was marked closed") + } + require.Len(t, f.comments["GHSA-dup"], 1, "the comment was posted again") + + f.closeErr = nil + list := f.list() + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", list)) + require.Equal(t, "closed", f.byID["GHSA-dup"].State) + require.Len(t, f.comments["GHSA-dup"], 1) + require.Equal(t, "closed", dup(list).State, "the following Sync would see the report still in triage") +} + +func TestCloseDuplicatesLeavesALoopOfDuplicatesAlone(t *testing.T) { + _, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + require.NoError(t, b.Put(t.Context(), github.Advisory{GHSAID: "GHSA-orig", Summary: "s", State: "triage", CreatedAt: reported}, Duplicate, &Triage{DuplicateOf: "GHSA-dup"}, "")) + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + + require.NoError(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list())) + + require.Zero(t, f.updates) + require.Empty(t, f.comments) +} + +func TestCloseDuplicatesReportsAFailedNoteOnceTheReportIsClosed(t *testing.T) { + p, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig", State: "triage"}) + p.IssueErr = errors.New("HTTP 502") + all := f.list() + + require.ErrorContains(t, b.CloseDuplicates(t.Context(), f, "php/php-src", all), "HTTP 502") + + require.Equal(t, "closed", f.byID["GHSA-dup"].State) + require.True(t, slices.ContainsFunc(all, func(a github.Advisory) bool { return a.GHSAID == "GHSA-dup" && a.State == "closed" }), "the Sync that follows sees the report in triage") + require.Empty(t, p.IssueOf(p.Find("GHSA-dup")).Comments) +} + +func TestCloseDuplicatesReportsAMissingOriginal(t *testing.T) { + _, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + f := newAdvisories(github.Advisory{GHSAID: "GHSA-orig"}) + delete(f.byID, "GHSA-orig") + + require.ErrorIs(t, b.CloseDuplicates(t.Context(), f, "php/php-src", f.list()), github.ErrNotFound) + require.Equal(t, "triage", f.byID["GHSA-dup"].State) + require.Empty(t, f.comments) +} + +func TestSyncKeepsTheIssueOfAClosedDuplicateClosedAsSuch(t *testing.T) { + p, b := closedAsDuplicate(t, "duplicate", "GHSA-orig") + + require.NoError(t, b.Sync(t.Context(), []github.Advisory{{GHSAID: "GHSA-orig", State: "closed"}, {GHSAID: "GHSA-dup", State: "closed"}})) + + require.Equal(t, "duplicate", p.IssueOf(p.Find("GHSA-dup")).StateReason) + require.Equal(t, "closed", p.Value(p.Find("GHSA-dup"), "State")) + require.Equal(t, "not_planned", p.IssueOf(p.Find("GHSA-orig")).StateReason) +} diff --git a/internal/board/boardtest/boardtest.go b/internal/board/boardtest/boardtest.go index cbbf8e3..e2797ee 100644 --- a/internal/board/boardtest/boardtest.go +++ b/internal/board/boardtest/boardtest.go @@ -255,7 +255,7 @@ func (p *Project) ListIssues(_ context.Context, repo string) ([]github.Issue, er var issues []github.Issue for n, i := range p.Issues { if i.Repo == repo { - issues = append(issues, github.Issue{ID: i.ID, Number: n, Title: i.Title, Body: i.Body}) + issues = append(issues, github.Issue{ID: i.ID, Number: n, Title: i.Title, Body: i.Body, State: i.State, StateReason: i.StateReason}) } } diff --git a/internal/github/client.go b/internal/github/client.go index 224fde9..bf49d13 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -59,6 +59,12 @@ type Advisory struct { Author struct { Login string `json:"login"` } `json:"author"` + Credits []Credit `json:"credits"` +} + +type Credit struct { + Login string `json:"login"` + Type string `json:"type"` } type Issue struct { @@ -67,6 +73,7 @@ type Issue struct { Title string `json:"title"` Body string `json:"body"` State string `json:"state"` + StateReason string `json:"state_reason"` HTMLURL string `json:"html_url"` PullRequest *struct{} `json:"pull_request"` } @@ -115,6 +122,14 @@ func (c *Client) ListAdvisories(ctx context.Context, repo, state string) ([]Advi return list[Advisory](ctx, c, fmt.Sprintf("%s/repos/%s/security-advisories?%s", c.BaseURL, repo, q.Encode())) } +// UpdateAdvisory sets the given fields of an advisory, such as "state" or "credits". GitHub asks for the whole +// list of credits, the ones already there included. +func (c *Client) UpdateAdvisory(ctx context.Context, repo, ghsa string, fields map[string]any) error { + _, err := c.do(ctx, http.MethodPatch, fmt.Sprintf("%s/repos/%s/security-advisories/%s", c.BaseURL, repo, url.PathEscape(ghsa)), fields, nil) + + return err +} + // ListComments relies on the advisory comments API, in public preview since 2026-10-02. func (c *Client) ListComments(ctx context.Context, repo, ghsa string) ([]Comment, error) { return list[Comment](ctx, c, fmt.Sprintf("%s/repos/%s/security-advisories/%s/comments?per_page=100", c.BaseURL, repo, url.PathEscape(ghsa))) diff --git a/internal/github/client_test.go b/internal/github/client_test.go index bf461a9..b83a6b7 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -205,7 +205,7 @@ func TestIssues(t *testing.T) { fmt.Fprint(w, `{"private":true}`) case "GET /repos/acme/private-issues/issues": assert.Equal(t, "all", r.URL.Query().Get("state"), "closed issues would be created again") - fmt.Fprint(w, `[{"id":42,"number":7,"body":"b"}]`) + fmt.Fprint(w, `[{"id":42,"number":7,"body":"b","state":"closed","state_reason":"duplicate"}]`) case "POST /repos/acme/private-issues/issues": assert.Equal(t, map[string]any{"title": "t", "body": "b"}, body) w.WriteHeader(http.StatusCreated) @@ -237,7 +237,7 @@ func TestIssues(t *testing.T) { require.True(t, repo.Private) issues, err := c.ListIssues(context.Background(), "acme/private-issues") require.NoError(t, err) - require.Equal(t, []Issue{{ID: 42, Number: 7, Body: "b"}}, issues) + require.Equal(t, []Issue{{ID: 42, Number: 7, Body: "b", State: "closed", StateReason: "duplicate"}}, issues) issue, err := c.CreateIssue(context.Background(), "acme/private-issues", "t", "b") require.NoError(t, err) require.Equal(t, int64(42), issue.ID) @@ -400,16 +400,40 @@ func TestNew(t *testing.T) { require.Positive(t, c.HTTP.Timeout) } +func TestUpdateAdvisory(t *testing.T) { + c := newTestClient(t, func(_ http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPatch, r.Method) + assert.Equal(t, "/repos/o/r/security-advisories/GHSA-1", r.URL.Path) + var body map[string]any + assert.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, map[string]any{"credits": []any{map[string]any{"login": "bob", "type": "reporter"}}}, body) + }) + require.NoError(t, c.UpdateAdvisory(t.Context(), "o/r", "GHSA-1", map[string]any{"credits": []Credit{{Login: "bob", Type: "reporter"}}})) + + c = newTestClient(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusForbidden) }) + var httpErr *HTTPError + require.ErrorAs(t, c.UpdateAdvisory(t.Context(), "o/r", "GHSA-1", map[string]any{"state": "closed"}), &httpErr) + require.Equal(t, http.StatusForbidden, httpErr.Status) +} + +func TestCommentIssueReportsARefusal(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusForbidden) }) + var httpErr *HTTPError + require.ErrorAs(t, c.CommentIssue(t.Context(), "acme/private-issues", 7, "c"), &httpErr) + require.Equal(t, http.StatusForbidden, httpErr.Status) +} + func TestGetAdvisoryReadsTheCurrentReportAndReporter(t *testing.T) { c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, http.MethodGet, r.Method) assert.Equal(t, "/repos/o/r/security-advisories/GHSA-1", r.URL.Path) - fmt.Fprint(w, `{"ghsa_id":"GHSA-1","description":"current revision","author":{"login":"reporter"}}`) + fmt.Fprint(w, `{"ghsa_id":"GHSA-1","description":"current revision","author":{"login":"reporter"},"credits":[{"login":"helper","type":"analyst"}]}`) }) a, err := c.GetAdvisory(t.Context(), "o/r", "GHSA-1") require.NoError(t, err) require.Equal(t, "current revision", a.Description) require.Equal(t, "reporter", a.Author.Login) + require.Equal(t, []Credit{{Login: "helper", Type: "analyst"}}, a.Credits) c = newTestClient(t, func(w http.ResponseWriter, _ *http.Request) { http.NotFound(w, nil) }) _, err = c.GetAdvisory(t.Context(), "o/r", "missing") require.ErrorIs(t, err, ErrNotFound, "missing advisory accepted") diff --git a/internal/triage/triage.go b/internal/triage/triage.go index d16b3cc..7b1671a 100644 --- a/internal/triage/triage.go +++ b/internal/triage/triage.go @@ -58,6 +58,7 @@ type GitHub interface { Login(ctx context.Context) (string, error) ListComments(ctx context.Context, repo, ghsa string) ([]github.Comment, error) PostComment(ctx context.Context, repo, ghsa, body string) error + UpdateAdvisory(ctx context.Context, repo, ghsa string, fields map[string]any) error } type Config struct { @@ -81,6 +82,8 @@ type Config struct { // writes them with a token that only reaches that repository. IssuesRepository string Issues board.Issues + // CloseDuplicates closes the advisory of an issue a maintainer closed as a duplicate, see board.CloseDuplicates. + CloseDuplicates bool // Packs names the built-in guidance packs appended to the reader's method, such as php-src. Packs []string // Force triages every selected advisory again for the board, even one its item already holds a triage for. @@ -165,6 +168,13 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E log.Error("the project board is left out of this run", "reason", boardErr.Error()) } } + // The issues were read when the board opened, so the duplicates close before the triage takes hours. + if b != nil && cfg.CloseDuplicates { + if err := b.CloseDuplicates(ctx, gh, cfg.Repo, all); err != nil { + log.Error("closing the duplicates failed", "reason", describe(err)) + boardErr = errors.Join(boardErr, errors.New("closing the duplicates: "+describe(err))) + } + } log.Info("advisories to check", "count", len(slices.DeleteFunc(slices.Clone(all), func(a github.Advisory) bool { return !selected(cfg, a) }))) // The first reference is copied once, then mounted read-only in every agent run of this execution. The @@ -283,7 +293,7 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E if b != nil { if err := b.Sync(ctx, all); err != nil { log.Error("syncing the project board failed", "reason", describe(err)) - boardErr = errors.New("syncing the project board: " + describe(err)) + boardErr = errors.Join(boardErr, errors.New("syncing the project board: "+describe(err))) } } log.Info("done", "triaged", triaged, "already_triaged", skipped, "failed", failed) diff --git a/internal/triage/triage_test.go b/internal/triage/triage_test.go index 93a4cc9..f7d13e6 100644 --- a/internal/triage/triage_test.go +++ b/internal/triage/triage_test.go @@ -44,6 +44,7 @@ type fakeGitHub struct { advisories []github.Advisory comments map[string][]github.Comment posted map[string]string + updated map[string][]map[string]any commentsErr error loginErr error listErr error @@ -93,6 +94,15 @@ func (f *fakeGitHub) ListComments(_ context.Context, _, ghsa string) ([]github.C return f.comments[ghsa], nil } +func (f *fakeGitHub) UpdateAdvisory(_ context.Context, _, ghsa string, fields map[string]any) error { + if f.updated == nil { + f.updated = map[string][]map[string]any{} + } + f.updated[ghsa] = append(f.updated[ghsa], fields) + + return nil +} + func (f *fakeGitHub) PostComment(_ context.Context, _, ghsa, body string) error { if f.postErr != nil { return f.postErr @@ -1549,6 +1559,33 @@ func TestRunPutsTheTriageInAPrivateIssue(t *testing.T) { require.Empty(t, gh.posted, "the board triage posted a comment") } +func TestRunClosesTheReportOfAnIssueClosedAsADuplicateOnlyWhenAsked(t *testing.T) { + for _, enabled := range []bool{false, true} { + gh, eng := newFixture(), &fakeEngine{} + cfg := Config{Advisory: "GHSA-done", Project: "acme/16", IssuesRepository: "acme/private-issues", CloseDuplicates: enabled} + require.NoError(t, run(t, cfg, gh, eng)) + id := gh.Find("GHSA-done") + for _, f := range gh.Fields { + if f.Name == "Duplicate of" { + gh.Items[id][f.ID] = "GHSA-old" + } + } + issue := gh.IssueOf(id) + issue.State, issue.StateReason = "closed", "duplicate" + + require.NoError(t, run(t, cfg, gh, eng)) + + if !enabled { + require.Empty(t, gh.updated, "an advisory changed without close-duplicates") + continue + } + require.Equal(t, []map[string]any{{"state": "closed"}}, gh.updated["GHSA-done"]) + require.Contains(t, gh.posted["GHSA-done"], "duplicate of GHSA-old") + require.Equal(t, "duplicate", issue.StateReason) + require.Equal(t, "closed", gh.Value(id, "State"), "the board kept the closed report in triage") + } +} + func TestRunNeverCreatesAnIssueInAPublicRepository(t *testing.T) { for name, repo := range map[string]string{"public": "php/php-src", "advisory repository": "o/r"} { t.Run(name, func(t *testing.T) {