From f2eb240ae937e46d6ef1e0dbe85b0463f703ec47 Mon Sep 17 00:00:00 2001 From: Virgil Date: Mon, 30 Mar 2026 10:34:08 +0000 Subject: [PATCH] fix(ax): tighten audit JSON and deterministic fetch errors --- cmd/qa/cmd_issues.go | 10 +++++++ cmd/qa/cmd_php_test.go | 61 ++++++++++++++++++++++++++++++++++++++++++ cmd/qa/cmd_review.go | 12 +++++++++ pkg/php/audit.go | 20 +++++++------- pkg/php/audit_test.go | 47 +++++++++++++++++++++++++------- 5 files changed, 132 insertions(+), 18 deletions(-) diff --git a/cmd/qa/cmd_issues.go b/cmd/qa/cmd_issues.go index 3975142..e100221 100644 --- a/cmd/qa/cmd_issues.go +++ b/cmd/qa/cmd_issues.go @@ -165,6 +165,16 @@ func runQAIssues() error { } allIssues = append(allIssues, issues...) } + + if len(fetchErrors) > 1 { + slices.SortFunc(fetchErrors, func(a, b IssueFetchError) int { + return cmp.Or( + cmp.Compare(a.Repo, b.Repo), + cmp.Compare(a.Error, b.Error), + ) + }) + } + totalIssues := len(allIssues) if len(allIssues) == 0 { diff --git a/cmd/qa/cmd_php_test.go b/cmd/qa/cmd_php_test.go index 97d4faa..1b0c2dc 100644 --- a/cmd/qa/cmd_php_test.go +++ b/cmd/qa/cmd_php_test.go @@ -153,6 +153,55 @@ func TestPHPSecuritySARIFOutput_IsStructuredAndChromeFree(t *testing.T) { assert.NotContains(t, output, "Summary:") } +func TestPHPAuditJSONOutput_UsesMachineFriendlyKeys(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "composer.json"), "{}") + writeExecutable(t, filepath.Join(dir, "composer"), `#!/bin/sh +case "$*" in + *"audit --format=json"*) + cat <<'JSON' +{"advisories":{"vendor/pkg":[{"title":"Remote Code Execution","link":"https://example.com/advisory/1","cve":"CVE-2026-0001","affectedVersions":">=1.0,<1.5"}]}} +JSON + exit 1 + ;; + *) + printf '%s\n' "unexpected composer invocation: $*" >&2 + exit 1 + ;; +esac +`) + + restoreWorkingDir(t, dir) + prependPath(t, dir) + resetPHPAuditFlags(t) + + parent := &cli.Command{Use: "qa"} + addPHPAuditCommand(parent) + command := findSubcommand(t, parent, "audit") + require.NoError(t, command.Flags().Set("json", "true")) + + output := captureStdout(t, func() { + require.Error(t, command.RunE(command, nil)) + }) + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(output), &payload)) + assert.Equal(t, true, payload["has_vulnerabilities"]) + + results := payload["results"].([]any) + require.Len(t, results, 1) + result := results[0].(map[string]any) + assert.Equal(t, "composer", result["tool"]) + + advisories := result["advisories"].([]any) + require.Len(t, advisories, 1) + advisory := advisories[0].(map[string]any) + assert.Equal(t, "vendor/pkg", advisory["package"]) + assert.Equal(t, ">=1.0,<1.5", advisory["affected_versions"]) + assert.NotContains(t, output, "Dependency Audit") + assert.NotContains(t, output, "Package") +} + func TestPHPTestJUnitOutput_PrintsOnlyXML(t *testing.T) { dir := t.TempDir() writeTestFile(t, filepath.Join(dir, "composer.json"), "{}") @@ -258,6 +307,18 @@ func resetPHPSecurityFlags(t *testing.T) { }) } +func resetPHPAuditFlags(t *testing.T) { + t.Helper() + oldJSON := phpAuditJSON + oldFix := phpAuditFix + phpAuditJSON = false + phpAuditFix = false + t.Cleanup(func() { + phpAuditJSON = oldJSON + phpAuditFix = oldFix + }) +} + func resetPHPTestFlags(t *testing.T) { t.Helper() oldParallel := phpTestParallel diff --git a/cmd/qa/cmd_review.go b/cmd/qa/cmd_review.go index cb1b5ea..7c85a4e 100644 --- a/cmd/qa/cmd_review.go +++ b/cmd/qa/cmd_review.go @@ -194,6 +194,18 @@ func runReview() error { } } + if len(fetchErrors) > 1 { + sort.Slice(fetchErrors, func(i, j int) bool { + if fetchErrors[i].Repo == fetchErrors[j].Repo { + if fetchErrors[i].Scope == fetchErrors[j].Scope { + return fetchErrors[i].Error < fetchErrors[j].Error + } + return fetchErrors[i].Scope < fetchErrors[j].Scope + } + return fetchErrors[i].Repo < fetchErrors[j].Repo + }) + } + if reviewJSON { data, err := json.MarshalIndent(reviewOutput{ Mine: minePRs, diff --git a/pkg/php/audit.go b/pkg/php/audit.go index 235c21a..e42ddf4 100644 --- a/pkg/php/audit.go +++ b/pkg/php/audit.go @@ -29,11 +29,12 @@ type AuditResult struct { // AuditAdvisory represents a single security advisory. type AuditAdvisory struct { - Package string - Severity string - Title string - URL string - Identifiers []string + Package string `json:"package"` + Severity string `json:"severity,omitempty"` + Title string `json:"title"` + URL string `json:"url,omitempty"` + Identifiers []string `json:"identifiers,omitempty"` + AffectedVersions string `json:"affected_versions,omitempty"` } // RunAudit runs security audits on dependencies. @@ -95,10 +96,11 @@ func runComposerAudit(ctx context.Context, opts AuditOptions) AuditResult { for pkg, advisories := range auditData.Advisories { for _, adv := range advisories { result.Advisories = append(result.Advisories, AuditAdvisory{ - Package: pkg, - Title: adv.Title, - URL: adv.Link, - Identifiers: []string{adv.CVE}, + Package: pkg, + Title: adv.Title, + URL: adv.Link, + Identifiers: []string{adv.CVE}, + AffectedVersions: adv.AffectedRanges, }) } } diff --git a/pkg/php/audit_test.go b/pkg/php/audit_test.go index bf1759f..805a10f 100644 --- a/pkg/php/audit_test.go +++ b/pkg/php/audit_test.go @@ -33,11 +33,12 @@ func TestAuditResult_Fields(t *testing.T) { func TestAuditAdvisory_Fields(t *testing.T) { adv := AuditAdvisory{ - Package: "laravel/framework", - Severity: "critical", - Title: "SQL Injection", - URL: "https://example.com/advisory", - Identifiers: []string{"CVE-2025-9999", "GHSA-xxxx"}, + Package: "laravel/framework", + Severity: "critical", + Title: "SQL Injection", + URL: "https://example.com/advisory", + Identifiers: []string{"CVE-2025-9999", "GHSA-xxxx"}, + AffectedVersions: ">=1.0,<2.0", } assert.Equal(t, "laravel/framework", adv.Package) @@ -45,6 +46,32 @@ func TestAuditAdvisory_Fields(t *testing.T) { assert.Equal(t, "SQL Injection", adv.Title) assert.Equal(t, "https://example.com/advisory", adv.URL) assert.Equal(t, []string{"CVE-2025-9999", "GHSA-xxxx"}, adv.Identifiers) + assert.Equal(t, ">=1.0,<2.0", adv.AffectedVersions) +} + +func TestAuditAdvisory_JSONUsesSnakeCase(t *testing.T) { + adv := AuditAdvisory{ + Package: "vendor/pkg", + Severity: "high", + Title: "Remote Code Execution", + URL: "https://example.com/advisory/1", + Identifiers: []string{"CVE-2025-0001"}, + AffectedVersions: ">=1.0,<1.5", + } + + data, err := json.Marshal(adv) + require.NoError(t, err) + + assert.JSONEq(t, `{ + "package": "vendor/pkg", + "severity": "high", + "title": "Remote Code Execution", + "url": "https://example.com/advisory/1", + "identifiers": ["CVE-2025-0001"], + "affected_versions": ">=1.0,<1.5" + }`, string(data)) + assert.NotContains(t, string(data), `"Package"`) + assert.NotContains(t, string(data), `"Identifiers"`) } func TestRunComposerAudit_ParsesJSON(t *testing.T) { @@ -94,10 +121,11 @@ func TestRunComposerAudit_ParsesJSON(t *testing.T) { for pkg, advisories := range auditData.Advisories { for _, adv := range advisories { result.Advisories = append(result.Advisories, AuditAdvisory{ - Package: pkg, - Title: adv.Title, - URL: adv.Link, - Identifiers: []string{adv.CVE}, + Package: pkg, + Title: adv.Title, + URL: adv.Link, + Identifiers: []string{adv.CVE}, + AffectedVersions: adv.AffectedRanges, }) } } @@ -117,6 +145,7 @@ func TestRunComposerAudit_ParsesJSON(t *testing.T) { assert.Equal(t, "Remote Code Execution", byPkg["vendor/package-a"][0].Title) assert.Equal(t, "https://example.com/advisory/1", byPkg["vendor/package-a"][0].URL) assert.Equal(t, []string{"CVE-2025-1234"}, byPkg["vendor/package-a"][0].Identifiers) + assert.Equal(t, ">=1.0,<1.5", byPkg["vendor/package-a"][0].AffectedVersions) assert.Len(t, byPkg["vendor/package-b"], 2) }