From c8f7a2fdac3bb88e47307c721c92e42676dab5a4 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Wed, 28 Oct 2026 08:43:21 +0100 Subject: [PATCH 1/2] Bound runs by time and fix two report rendering errors A run now starts an advisory only while 30 minutes remain of its 3 hours 50 minutes, which fits a 4-hour job, instead of stopping after 10 advisories. A failed duplicate search now says so instead of rendering "None found among the repository advisories.". A policy point ending with a period no longer renders as "point.: reason". --- README.md | 2 +- docs/configuration.md | 4 ++-- internal/report/report.go | 11 ++++++++--- internal/report/report_test.go | 18 ++++++++++++++++++ internal/triage/revisions.go | 2 +- internal/triage/triage.go | 11 +++++++---- internal/triage/triage_test.go | 22 ++++++++++++++++++---- 7 files changed, 55 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index b6bb09e..64da83b 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ concurrency: jobs: triage: runs-on: ubuntu-latest - timeout-minutes: 120 + timeout-minutes: 240 environment: tamis permissions: contents: read diff --git a/docs/configuration.md b/docs/configuration.md index 279f783..0ba0783 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -31,9 +31,9 @@ The private artifact contains a full `.md` report with every duplicate proposed ## Resource budgets -An execution permits at most 800 provider requests and 256 MiB of cumulative request bodies. Each advisory shares 200 requests and 64 MiB across its reader run and comparisons; each agent token additionally permits 150 requests, not counting the refused routes Codex calls on every run. A request body cannot exceed 1 MiB. API responses request at most 16,000 output tokens; the ChatGPT backend has its own output limits. The thirty-minute advisory deadline also cancels host-side git commands. At most ten unpublished exact candidates are separately confirmed. At most ten reports and one report per known reporter are attempted per execution, and a report is only started while the execution still has a whole advisory budget, otherwise it waits for the next execution. SDK retries are disabled. +An execution permits at most 800 provider requests and 256 MiB of cumulative request bodies. Each advisory shares 200 requests and 64 MiB across its reader run and comparisons; each agent token additionally permits 150 requests, not counting the refused routes Codex calls on every run. A request body cannot exceed 1 MiB. API responses request at most 16,000 output tokens; the ChatGPT backend has its own output limits. The thirty-minute advisory deadline also cancels host-side git commands. At most ten unpublished exact candidates are separately confirmed. At most one report per known reporter is attempted per execution, and a report is only started while the execution still has a whole advisory budget and thirty minutes left of its 3 hours 50 minutes, otherwise it waits for the next execution. SDK retries are disabled. -These bounds limit each execution, not spending across scheduled runs. Use a dedicated provider account/project with an enforced spending or credit ceiling, restrict models, disable hosted tools and network allowlists there, and configure billing alerts. Provider pricing, tokenisation and ChatGPT credit policies differ, so byte/request limits are not a monetary guarantee. Keep a workflow `timeout-minutes` and a single concurrency group. Budget exhaustion leaves a report retriable rather than marking it permanently complete. +These bounds limit each execution, not spending across scheduled runs. Use a dedicated provider account/project with an enforced spending or credit ceiling, restrict models, disable hosted tools and network allowlists there, and configure billing alerts. Provider pricing, tokenisation and ChatGPT credit policies differ, so byte/request limits are not a monetary guarantee. Keep a workflow `timeout-minutes` of at least 240 and a single concurrency group. Budget exhaustion leaves a report retriable rather than marking it permanently complete. ## Project guidance diff --git a/internal/report/report.go b/internal/report/report.go index 413d1c2..dae6de1 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -63,6 +63,8 @@ type Report struct { // Duplicates come from the comparator, never from the reader. Duplicates []Duplicate `json:"-"` + // DuplicateSearchFailed means Duplicates is empty because the comparator did not finish. + DuplicateSearchFailed bool `json:"-"` // Policies are the files the policy check read, empty when the repository has none. Policies []string `json:"-"` // CreatedAt is when the advisory was reported. @@ -486,9 +488,12 @@ func RenderPrivate(r Report) string { b.WriteString(text(r.Summary) + "\n") b.WriteString("\n#### Possible duplicates\n\n") - if len(r.Duplicates) == 0 { + switch { + case r.DuplicateSearchFailed: + b.WriteString("The duplicate search could not complete.\n") + case len(r.Duplicates) == 0: b.WriteString("None found among the repository advisories.\n") - } else { + default: b.WriteString("| Advisory | Status | Confidence |\n|---|---|---|\n") } // Starting from the report's own date leaves every duplicate unmarked when the report is the oldest. @@ -546,7 +551,7 @@ func RenderPrivate(r Report) string { b.WriteString("Nothing in the policy seems to exclude this report.\n") } for _, p := range r.PolicyExclusions { - fmt.Fprintf(&b, "- %s: %s\n", text(p.Point), text(p.Reason)) + fmt.Fprintf(&b, "- %s: %s\n", strings.TrimSuffix(text(p.Point), "."), text(p.Reason)) } } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 7b20130..85b09ba 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -323,6 +323,24 @@ func TestRenderPolicyWithoutExclusion(t *testing.T) { assert.Contains(t, got, "Checked against `.github/SECURITY.md`, `AGENTS.md`.\n\nNothing in the policy seems to exclude this report.") } +func TestRenderPolicyPointEndingWithAPeriod(t *testing.T) { + got := Render(Report{Policies: []string{"SECURITY.md"}, PolicyExclusions: []PolicyPoint{ + {Point: "Malicious inputs are not security issues.", Reason: "The PoC feeds `unserialize()`."}, + {Point: "Version 1.2 is unsupported", Reason: "It targets `1.2`."}, + }}) + + assert.Contains(t, got, "- Malicious inputs are not security issues: The PoC feeds `unserialize()`.\n") + assert.Contains(t, got, "- Version 1.2 is unsupported: It targets `1.2`.\n") + assert.NotContains(t, got, ".:") +} + +func TestRenderFailedDuplicateSearch(t *testing.T) { + got := Render(Report{Summary: "s", DuplicateSearchFailed: true}) + + assert.Contains(t, got, "#### Possible duplicates\n\nThe duplicate search could not complete.\n") + assert.NotContains(t, got, "None found among the repository advisories.") +} + func TestRenderEmpty(t *testing.T) { got := Render(Report{Summary: "Nothing"}) diff --git a/internal/triage/revisions.go b/internal/triage/revisions.go index 7b6ba06..ae52369 100644 --- a/internal/triage/revisions.go +++ b/internal/triage/revisions.go @@ -111,7 +111,7 @@ func triageRevisions(ctx context.Context, cfg Config, gh GitHub, reader Reader, duplicates, err := findDuplicates(ctx, path, reader, r, input, others, log) if err != nil { comparisonErr = err - privateNote = "\nThe duplicate search could not complete.\n" + r.DuplicateSearchFailed = true } else { matches := report.Report{Duplicates: duplicates} if err := (report.Checker{Advisories: byID}).Verify(ctx, &matches); err != nil { diff --git a/internal/triage/triage.go b/internal/triage/triage.go index 364f3f0..ef07be8 100644 --- a/internal/triage/triage.go +++ b/internal/triage/triage.go @@ -34,8 +34,10 @@ import ( // ProfilePath holds project-specific guidance, relative to the repository root. const ProfilePath = ".github/tamis.md" +// maxRunDuration leaves 10 minutes of a 4-hour job to its setup and to the upload of the private reports. +var maxRunDuration = 3*time.Hour + 50*time.Minute + const ( - maxTriagesPerRun = 10 maxAdvisoryDuration = 30 * time.Minute maxComparatorRequests = 200 maxAttempts = 3 @@ -106,7 +108,8 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E defer func() { _ = exec.Command("git", "-C", cfg.RepoPath, "worktree", "prune").Run() }() defer os.RemoveAll(sourceDir) - var triaged, skipped, failed, attempted int + var triaged, skipped, failed int + deadline := time.Now().Add(maxRunDuration) reporters := map[string]bool{} for _, a := range all { if !selected(cfg, a) { @@ -130,8 +133,9 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E skipped++ continue } - if attempted == maxTriagesPerRun { + if time.Until(deadline) < maxAdvisoryDuration { cancel() + log.Info("the run time is spent, the remaining advisories wait for the next run") break } if a.Author.Login != "" && reporters[strings.ToLower(a.Author.Login)] { @@ -146,7 +150,6 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E break } reporters[strings.ToLower(a.Author.Login)] = true - attempted++ log.Info("triaging", "attempt", failures+1) advisoryCtx, spent := usage.Track(engine.WithRequestLimit(advisoryCtx, maxComparatorRequests)) diff --git a/internal/triage/triage_test.go b/internal/triage/triage_test.go index a8c4b0c..7e2581e 100644 --- a/internal/triage/triage_test.go +++ b/internal/triage/triage_test.go @@ -727,18 +727,32 @@ func TestRunRejectsADuplicateOutsideTheAdvisories(t *testing.T) { body := gh.posted["GHSA-new"] require.Contains(t, body, report.Marker, "a failed duplicate search blocked the comment") require.NotContains(t, body, "GHSA-invented") + require.Contains(t, body, "The duplicate search could not complete.") + require.NotContains(t, body, "None found among the repository advisories.") require.Contains(t, privateReports(t, gh), "The duplicate search could not complete.") } -func TestRunCapsNewAdvisoriesPerExecution(t *testing.T) { +func TestRunTriagesEveryNewAdvisoryWithinItsTime(t *testing.T) { gh, eng := newFixture(), &fakeEngine{reader: `{"summary":"s"}`} gh.advisories = nil - for i := 0; i < maxTriagesPerRun+1; i++ { + for i := range 11 { gh.advisories = append(gh.advisories, github.Advisory{GHSAID: fmt.Sprintf("GHSA-%d", i), State: "triage", CreatedAt: since.Add(time.Hour)}) } require.NoError(t, run(t, Config{Since: since}, gh, eng)) - require.Len(t, gh.posted, maxTriagesPerRun) - require.Len(t, eng.role(""), maxTriagesPerRun) + require.Len(t, gh.posted, 11) + require.Len(t, eng.role(""), 11) +} + +func TestRunLeavesTheAdvisoriesToTheNextRunOnceItsTimeIsSpent(t *testing.T) { + defer func(d time.Duration) { maxRunDuration = d }(maxRunDuration) + maxRunDuration = maxAdvisoryDuration + gh, eng := newFixture(), &fakeEngine{} + var logs strings.Builder + + require.NoError(t, runLogged(t, Config{Since: since}, gh, eng, &logs)) + require.Empty(t, gh.posted) + require.Empty(t, eng.role("")) + require.Contains(t, logs.String(), "the run time is spent, the remaining advisories wait for the next run") } func TestRunHidesPartialMatchesOnUnpublishedAdvisories(t *testing.T) { From 664f3a0284553a59236a30e433a0ffac61443adf Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Wed, 28 Oct 2026 08:50:51 +0100 Subject: [PATCH 2/2] Retry a failed agent run once An agent run that fails without a valid report is run again after a minute, so a model provider briefly overloaded or dropping its stream no longer costs a triage attempt or publishes a report without its duplicate search. --- docs/configuration.md | 2 +- internal/triage/triage.go | 28 ++++++++++++++++-- internal/triage/triage_test.go | 54 +++++++++++++++++++++++++++++++++- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 0ba0783..e4ce79d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -31,7 +31,7 @@ The private artifact contains a full `.md` report with every duplicate proposed ## Resource budgets -An execution permits at most 800 provider requests and 256 MiB of cumulative request bodies. Each advisory shares 200 requests and 64 MiB across its reader run and comparisons; each agent token additionally permits 150 requests, not counting the refused routes Codex calls on every run. A request body cannot exceed 1 MiB. API responses request at most 16,000 output tokens; the ChatGPT backend has its own output limits. The thirty-minute advisory deadline also cancels host-side git commands. At most ten unpublished exact candidates are separately confirmed. At most one report per known reporter is attempted per execution, and a report is only started while the execution still has a whole advisory budget and thirty minutes left of its 3 hours 50 minutes, otherwise it waits for the next execution. SDK retries are disabled. +An execution permits at most 800 provider requests and 256 MiB of cumulative request bodies. Each advisory shares 200 requests and 64 MiB across its reader run and comparisons; each agent token additionally permits 150 requests, not counting the refused routes Codex calls on every run. A request body cannot exceed 1 MiB. API responses request at most 16,000 output tokens; the ChatGPT backend has its own output limits. The thirty-minute advisory deadline also cancels host-side git commands. At most ten unpublished exact candidates are separately confirmed. At most one report per known reporter is attempted per execution, and a report is only started while the execution still has a whole advisory budget and thirty minutes left of its 3 hours 50 minutes, otherwise it waits for the next execution. SDK retries are disabled, and an agent run that fails without a valid report is retried once after a minute. These bounds limit each execution, not spending across scheduled runs. Use a dedicated provider account/project with an enforced spending or credit ceiling, restrict models, disable hosted tools and network allowlists there, and configure billing alerts. Provider pricing, tokenisation and ChatGPT credit policies differ, so byte/request limits are not a monetary guarantee. Keep a workflow `timeout-minutes` of at least 240 and a single concurrency group. Budget exhaustion leaves a report retriable rather than marking it permanently complete. diff --git a/internal/triage/triage.go b/internal/triage/triage.go index ef07be8..6fdf7cc 100644 --- a/internal/triage/triage.go +++ b/internal/triage/triage.go @@ -34,7 +34,6 @@ import ( // ProfilePath holds project-specific guidance, relative to the repository root. const ProfilePath = ".github/tamis.md" -// maxRunDuration leaves 10 minutes of a 4-hour job to its setup and to the upload of the private reports. var maxRunDuration = 3*time.Hour + 50*time.Minute const ( @@ -78,6 +77,31 @@ type Reader interface { Read(ctx context.Context, req sandbox.Request) (json.RawMessage, error) } +var retryDelay = time.Minute + +// retrying runs a failed agent once more after retryDelay, so a model provider briefly overloaded or dropping +// its stream does not cost a triage attempt. +type retrying struct { + Reader + log *slog.Logger +} + +func (r retrying) Read(ctx context.Context, req sandbox.Request) (json.RawMessage, error) { + raw, err := r.Reader.Read(ctx, req) + // An incomplete run already wrote a valid report, which the triage keeps. + if !errors.As(err, new(*sandbox.Error)) || errors.As(err, new(*sandbox.Incomplete)) { + return raw, err + } + r.log.Info("the agent run failed, retrying it", "delay", retryDelay) + select { + case <-ctx.Done(): + return raw, err + case <-time.After(retryDelay): + } + + return r.Reader.Read(ctx, req) +} + // Run logs which advisory it works on and its steps, never report content nor raw errors. The action only // runs from a private repository, so its logs stay with the security team. func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.Engine, log *slog.Logger) error { @@ -153,7 +177,7 @@ func Run(ctx context.Context, cfg Config, gh GitHub, reader Reader, eng engine.E log.Info("triaging", "attempt", failures+1) advisoryCtx, spent := usage.Track(engine.WithRequestLimit(advisoryCtx, maxComparatorRequests)) - if published, err := triageRevisions(advisoryCtx, cfg, gh, reader, eng, a, all, comments, login, revision, failures+1 >= maxAttempts, sourceDir, log); err != nil { + if published, err := triageRevisions(advisoryCtx, cfg, gh, retrying{reader, log}, eng, a, all, comments, login, revision, failures+1 >= maxAttempts, sourceDir, log); err != nil { cancel() log.Error("triage failed", "reason", describe(err), "usage", spent()) if keepAgentError(cfg, a.GHSAID, revision, err) != nil { diff --git a/internal/triage/triage_test.go b/internal/triage/triage_test.go index 7e2581e..a81cdc0 100644 --- a/internal/triage/triage_test.go +++ b/internal/triage/triage_test.go @@ -114,6 +114,8 @@ type fakeEngine struct { fail string // crash makes fail look like an agent container exiting on an error. crash bool + // failures is how many calls matching fail fail before the next ones succeed, all of them when zero. + failures, failed int // incomplete makes the reader hand back its report with a failed agent run. incomplete bool reader string @@ -146,7 +148,8 @@ func (e *fakeEngine) Read(ctx context.Context, req sandbox.Request) (json.RawMes } } e.calls = append(e.calls, call) - if e.fail != "" && strings.Contains(call.prompt, e.fail) { + if e.fail != "" && strings.Contains(call.prompt, e.fail) && (e.failures == 0 || e.failed < e.failures) { + e.failed++ err := errors.New("model unavailable while reading SECRET-PAYLOAD from " + call.prompt) if e.crash { return nil, &sandbox.Error{Err: err} @@ -807,7 +810,56 @@ func TestRunReportsFailuresAndContinues(t *testing.T) { require.NotEmpty(t, gh.posted["GHSA-other"], "the other advisory should still be triaged") } +func noRetryDelay(t *testing.T) { + t.Helper() + delay := retryDelay + retryDelay = 0 + t.Cleanup(func() { retryDelay = delay }) +} + +func TestRunRetriesAFailedAgentRunOnce(t *testing.T) { + noRetryDelay(t) + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: true, failures: 1} + var logs bytes.Buffer + + require.NoError(t, runLogged(t, Config{Advisory: "GHSA-new"}, gh, eng, &logs)) + + require.Len(t, eng.role(""), 2) + require.Contains(t, gh.posted["GHSA-new"], report.Marker) + require.Contains(t, gh.posted["GHSA-new"], "Looks like a heap overflow.") + require.Contains(t, logs.String(), "the agent run failed, retrying it") +} + +func TestRunFailsWhenTheRetriedAgentRunFailsAgain(t *testing.T) { + noRetryDelay(t) + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: true} + + require.Error(t, run(t, Config{Advisory: "GHSA-new"}, gh, eng)) + + require.Len(t, eng.role(""), 2, "a failed agent run is retried once only") + require.Contains(t, gh.posted["GHSA-new"], report.FailedMarker) +} + +func TestRunDoesNotRetryAFailureOutsideTheAgentRun(t *testing.T) { + gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new"} + var logs bytes.Buffer + + require.Error(t, runLogged(t, Config{Advisory: "GHSA-new"}, gh, eng, &logs)) + + require.Len(t, eng.role(""), 1) + require.NotContains(t, logs.String(), "retrying") +} + +func TestRunDoesNotRetryAnIncompleteAgentRun(t *testing.T) { + gh, eng := newFixture(), &fakeEngine{incomplete: true} + + require.Error(t, run(t, Config{Advisory: "GHSA-new"}, gh, eng)) + + require.Len(t, eng.role(""), 1) +} + func TestRunKeepsTheAgentErrorInThePrivateReports(t *testing.T) { + noRetryDelay(t) for name, crash := range map[string]bool{"agent crash": true, "other failure": false} { t.Run(name, func(t *testing.T) { gh, eng := newFixture(), &fakeEngine{fail: "GHSA-new", crash: crash}