From 7b2bb529e1437990d0adfc32cecebcab7e439b02 Mon Sep 17 00:00:00 2001 From: Virgil Date: Mon, 30 Mar 2026 12:11:28 +0000 Subject: [PATCH] fix(ax): honour php security flags --- cmd/qa/cmd_php_test.go | 41 ++++++++++ pkg/php/security.go | 157 ++++++++++++++++++++++++++++++++++----- pkg/php/security_test.go | 93 ++++++++++++++++++++++- 3 files changed, 273 insertions(+), 18 deletions(-) diff --git a/cmd/qa/cmd_php_test.go b/cmd/qa/cmd_php_test.go index f6c9fc2..1019f1b 100644 --- a/cmd/qa/cmd_php_test.go +++ b/cmd/qa/cmd_php_test.go @@ -153,6 +153,47 @@ func TestPHPSecuritySARIFOutput_IsStructuredAndChromeFree(t *testing.T) { assert.NotContains(t, output, "Summary:") } +func TestPHPSecurityJSONOutput_RespectsSeverityFilter(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "composer.json"), "{}") + writeTestFile(t, filepath.Join(dir, ".env"), "APP_DEBUG=true\nAPP_KEY=short\nAPP_URL=http://example.com\n") + writeExecutable(t, filepath.Join(dir, "bin", "composer"), "#!/bin/sh\nprintf '%s\\n' '{\"advisories\":{}}'\n") + + restoreWorkingDir(t, dir) + prependPath(t, filepath.Join(dir, "bin")) + resetPHPSecurityFlags(t) + + parent := &cli.Command{Use: "qa"} + addPHPSecurityCommand(parent) + command := findSubcommand(t, parent, "security") + require.NoError(t, command.Flags().Set("json", "true")) + require.NoError(t, command.Flags().Set("severity", "critical")) + + output := captureStdout(t, func() { + require.Error(t, command.RunE(command, nil)) + }) + + var payload struct { + Checks []struct { + ID string `json:"id"` + Severity string `json:"severity"` + } `json:"checks"` + Summary struct { + Total int `json:"total"` + Passed int `json:"passed"` + Critical int `json:"critical"` + High int `json:"high"` + } `json:"summary"` + } + require.NoError(t, json.Unmarshal([]byte(output), &payload)) + assert.Equal(t, 3, payload.Summary.Total) + assert.Equal(t, 1, payload.Summary.Passed) + assert.Equal(t, 2, payload.Summary.Critical) + assert.Zero(t, payload.Summary.High) + require.Len(t, payload.Checks, 3) + assert.NotContains(t, output, "https_enforced") +} + func TestPHPAuditJSONOutput_UsesLowerCaseAdvisoryKeys(t *testing.T) { dir := t.TempDir() writeTestFile(t, filepath.Join(dir, "composer.json"), "{}") diff --git a/pkg/php/security.go b/pkg/php/security.go index 2d729cc..0f14557 100644 --- a/pkg/php/security.go +++ b/pkg/php/security.go @@ -4,10 +4,14 @@ import ( "cmp" "context" "fmt" + "io" + "net/http" + "net/url" "os" "path/filepath" "slices" "strings" + "time" coreio "forge.lthn.ai/core/go-io" coreerr "forge.lthn.ai/core/go-log" @@ -58,6 +62,50 @@ func capitalise(s string) string { return strings.ToUpper(s[:1]) + s[1:] } +// securitySeverityRank maps severities to a sortable rank. +// Lower numbers are more severe. +func securitySeverityRank(severity string) (int, bool) { + switch strings.ToLower(strings.TrimSpace(severity)) { + case "critical": + return 0, true + case "high": + return 1, true + case "medium": + return 2, true + case "low": + return 3, true + case "info": + return 4, true + default: + return 0, false + } +} + +// filterSecurityChecks returns checks at or above the requested severity. +func filterSecurityChecks(checks []SecurityCheck, minimum string) ([]SecurityCheck, error) { + if strings.TrimSpace(minimum) == "" { + return checks, nil + } + + minRank, ok := securitySeverityRank(minimum) + if !ok { + return nil, coreerr.E("filterSecurityChecks", "invalid security severity "+minimum, nil) + } + + filtered := make([]SecurityCheck, 0, len(checks)) + for _, check := range checks { + rank, ok := securitySeverityRank(check.Severity) + if !ok { + continue + } + if rank <= minRank { + filtered = append(filtered, check) + } + } + + return filtered, nil +} + // RunSecurityChecks runs security checks on the project. func RunSecurityChecks(ctx context.Context, opts SecurityOptions) (*SecurityResult, error) { if opts.Dir == "" { @@ -95,24 +143,14 @@ func RunSecurityChecks(ctx context.Context, opts SecurityOptions) (*SecurityResu fsChecks := runFilesystemSecurityChecks(opts.Dir) result.Checks = append(result.Checks, fsChecks...) - // Calculate summary - for _, check := range result.Checks { - result.Summary.Total++ - if check.Passed { - result.Summary.Passed++ - } else { - switch check.Severity { - case "critical": - result.Summary.Critical++ - case "high": - result.Summary.High++ - case "medium": - result.Summary.Medium++ - case "low": - result.Summary.Low++ - } - } + // Check HTTP security headers when a URL is supplied. + result.Checks = append(result.Checks, runHTTPSecurityHeaderChecks(ctx, opts.URL)...) + + filteredChecks, err := filterSecurityChecks(result.Checks, opts.Severity) + if err != nil { + return nil, err } + result.Checks = filteredChecks // Keep the check order stable for callers that consume the package result // directly instead of going through the CLI layer. @@ -120,9 +158,94 @@ func RunSecurityChecks(ctx context.Context, opts SecurityOptions) (*SecurityResu return cmp.Compare(a.ID, b.ID) }) + // Calculate summary after any severity filtering has been applied. + for _, check := range result.Checks { + result.Summary.Total++ + if check.Passed { + result.Summary.Passed++ + continue + } + + switch check.Severity { + case "critical": + result.Summary.Critical++ + case "high": + result.Summary.High++ + case "medium": + result.Summary.Medium++ + case "low": + result.Summary.Low++ + } + } + return result, nil } +func runHTTPSecurityHeaderChecks(ctx context.Context, rawURL string) []SecurityCheck { + if strings.TrimSpace(rawURL) == "" { + return nil + } + + check := SecurityCheck{ + ID: "http_security_headers", + Name: "HTTP Security Headers", + Description: "Check for common security headers on the supplied URL", + Severity: "high", + CWE: "CWE-693", + } + + parsedURL, err := url.Parse(rawURL) + if err != nil || parsedURL.Scheme == "" || parsedURL.Host == "" { + check.Message = "Invalid URL" + check.Fix = "Provide a valid http:// or https:// URL" + return []SecurityCheck{check} + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + check.Message = err.Error() + check.Fix = "Provide a reachable URL" + return []SecurityCheck{check} + } + + client := &http.Client{Timeout: 10 * time.Second} + resp, err := client.Do(req) + if err != nil { + check.Message = err.Error() + check.Fix = "Ensure the URL is reachable" + return []SecurityCheck{check} + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) + + requiredHeaders := []string{ + "Content-Security-Policy", + "X-Frame-Options", + "X-Content-Type-Options", + "Referrer-Policy", + } + if strings.EqualFold(parsedURL.Scheme, "https") { + requiredHeaders = append(requiredHeaders, "Strict-Transport-Security") + } + + var missing []string + for _, header := range requiredHeaders { + if strings.TrimSpace(resp.Header.Get(header)) == "" { + missing = append(missing, header) + } + } + + if len(missing) == 0 { + check.Passed = true + check.Message = "Common security headers are present" + return []SecurityCheck{check} + } + + check.Message = fmt.Sprintf("Missing headers: %s", strings.Join(missing, ", ")) + check.Fix = "Add the missing security headers to the response" + return []SecurityCheck{check} +} + func runEnvSecurityChecks(dir string) []SecurityCheck { var checks []SecurityCheck diff --git a/pkg/php/security_test.go b/pkg/php/security_test.go index 5dee9ed..94dbce9 100644 --- a/pkg/php/security_test.go +++ b/pkg/php/security_test.go @@ -2,8 +2,11 @@ package php import ( "context" + "net/http" + "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -192,7 +195,7 @@ func TestRunSecurityChecks_Summary(t *testing.T) { // Summary should have totals assert.Greater(t, result.Summary.Total, 0) assert.Greater(t, result.Summary.Critical, 0) // at least debug_mode fails - assert.Greater(t, result.Summary.High, 0) // at least https_enforced fails + assert.Greater(t, result.Summary.High, 0) // at least https_enforced fails } func TestRunSecurityChecks_DefaultsDir(t *testing.T) { @@ -202,9 +205,97 @@ func TestRunSecurityChecks_DefaultsDir(t *testing.T) { assert.NotNil(t, result) } +func TestRunSecurityChecks_SeverityFilterCritical(t *testing.T) { + dir := t.TempDir() + setupSecurityFixture(t, dir, "APP_DEBUG=true\nAPP_KEY=short\nAPP_URL=http://example.com\n") + + result, err := RunSecurityChecks(context.Background(), SecurityOptions{ + Dir: dir, + Severity: "critical", + }) + require.NoError(t, err) + + require.Len(t, result.Checks, 3) + assert.Equal(t, 3, result.Summary.Total) + assert.Equal(t, 1, result.Summary.Passed) + assert.Equal(t, 2, result.Summary.Critical) + assert.Zero(t, result.Summary.High) + + for _, check := range result.Checks { + assert.Equal(t, "critical", check.Severity) + } + + byID := make(map[string]SecurityCheck) + for _, check := range result.Checks { + byID[check.ID] = check + } + + assert.NotContains(t, byID, "https_enforced") + assert.Contains(t, byID, "app_key_set") + assert.Contains(t, byID, "composer_audit") + assert.Contains(t, byID, "debug_mode") +} + +func TestRunSecurityChecks_URLAddsHeaderCheck(t *testing.T) { + dir := t.TempDir() + setupSecurityFixture(t, dir, "APP_DEBUG=false\nAPP_KEY=base64:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa=\nAPP_URL=https://example.com\n") + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-Content-Type-Options", "nosniff") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + })) + defer server.Close() + + result, err := RunSecurityChecks(context.Background(), SecurityOptions{ + Dir: dir, + URL: server.URL, + }) + require.NoError(t, err) + + byID := make(map[string]SecurityCheck) + for _, check := range result.Checks { + byID[check.ID] = check + } + + headerCheck, ok := byID["http_security_headers"] + require.True(t, ok) + assert.False(t, headerCheck.Passed) + assert.Equal(t, "high", headerCheck.Severity) + assert.True(t, strings.Contains(headerCheck.Message, "Missing headers")) + assert.NotEmpty(t, headerCheck.Fix) + + assert.Equal(t, 5, result.Summary.Total) + assert.Equal(t, 4, result.Summary.Passed) + assert.Equal(t, 1, result.Summary.High) +} + +func TestRunSecurityChecks_InvalidSeverity(t *testing.T) { + dir := t.TempDir() + + _, err := RunSecurityChecks(context.Background(), SecurityOptions{ + Dir: dir, + Severity: "banana", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid security severity") +} + func TestCapitalise(t *testing.T) { assert.Equal(t, "Composer", capitalise("composer")) assert.Equal(t, "Npm", capitalise("npm")) assert.Equal(t, "", capitalise("")) assert.Equal(t, "A", capitalise("a")) } + +func setupSecurityFixture(t *testing.T, dir string, envContent string) { + t.Helper() + + require.NoError(t, os.WriteFile(filepath.Join(dir, ".env"), []byte(envContent), 0o644)) + + composerBin := filepath.Join(dir, "composer") + require.NoError(t, os.WriteFile(composerBin, []byte("#!/bin/sh\ncat <<'JSON'\n{\"advisories\":{}}\nJSON\n"), 0o755)) + + oldPath := os.Getenv("PATH") + t.Setenv("PATH", dir+string(os.PathListSeparator)+oldPath) +}