diff --git a/internal/commands/agenthooks/cx/hooks.go b/internal/commands/agenthooks/cx/hooks.go index 12b043c2e..9fb7e3317 100644 --- a/internal/commands/agenthooks/cx/hooks.go +++ b/internal/commands/agenthooks/cx/hooks.go @@ -11,7 +11,9 @@ import ( "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/asca" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/kics" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/secrets" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/sca" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/scannerconfig" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/sessiontally" "github.com/checkmarx/ast-cli/internal/wrappers" ) @@ -19,9 +21,10 @@ import ( // canonical per-engine labels for the session summary (normalizes the legacy SCA/Oss split used by // the per-event remediation telemetry; ASCA already uses "Asca"). const ( - engineAsca = "Asca" - engineSca = "Sca" - engineKics = "Kics" + engineAsca = "Asca" + engineSca = "Sca" + engineKics = "Kics" + engineSecrets = "Secrets" ) // scaScanner is the package-level SCA scanner used by the guardrail handlers. @@ -33,6 +36,21 @@ var scaScanner *sca.Scanner // It is set by RegisterGuardrails and cleared by RegisterPassThrough. var kicsScanner *kics.Scanner +// bindThreshold points each guardrail's finding filter at the severity threshold for this event's +// agent, so findings below cx_severity_threshold are left out of a deny and the agent is not told to +// fix them. Agents the switches do not apply to keep every finding. +func bindThreshold(agent agenthooks.AgentID) { + below := func(severity string) bool { return belowThreshold(agent, severity) } + secrets.BelowThreshold = below + asca.BelowThreshold = below + kics.BelowThreshold = below + sca.BelowThreshold = below +} + +// secretsScanner is the package-level secret-detection scanner used by the file-edit guardrail. +// It is set by RegisterGuardrails and cleared by RegisterPassThrough. +var secretsScanner *secrets.Scanner + var telemetryWrapper wrappers.TelemetryWrapper func cxWhenAgentIdle(_ agenthooks.AgentIdleEvent) agenthooks.IdleVerdict { @@ -53,6 +71,7 @@ func sessionIDFromToolCall(ev *agenthooks.ToolCallEvent) string { // cxBeforeToolCall gates shell execution against the organization's blacklist, // tool rules, and the SCA guardrail (malicious / vulnerable package installs). func cxBeforeToolCall(ev agenthooks.ToolCallEvent) agenthooks.ToolVerdict { + bindThreshold(ev.Agent) if !ev.IsShell() { return agenthooks.Allow() } @@ -63,10 +82,10 @@ func cxBeforeToolCall(ev agenthooks.ToolCallEvent) agenthooks.ToolVerdict { } return agenthooks.Deny(reason) } - if scaScanner != nil { + if scaScanner != nil && scannerOn(ev.Agent, scannerconfig.SCA) { agent := agentToString(ev.Agent) sid := sessionIDFromToolCall(&ev) - if finding, remediation, severity := scaScanner.CheckBashInstall(ev.Command, ev.WorkDir, agent, sid); finding != "" { + if finding, remediation, severity := scaScanner.CheckBashInstall(ev.Command, ev.WorkDir, agent, sid); finding != "" && !belowThreshold(ev.Agent, severity) { sessiontally.Add(sid, engineSca, 1, 1) logRemediationTelemetry(agent, "SCA", severity, sid) return agenthooks.DenyWithContext(finding, remediation) @@ -81,10 +100,11 @@ func cxBeforeToolCall(ev agenthooks.ToolCallEvent) agenthooks.ToolVerdict { // 1. File EDITS (Claude / Windsurf / Droid / Gemini) — ev.Changes is populated. // Enforce blast_radius_limit, files_limits.max_total_file_size_kb, the ASCA // guardrail (AI-introduced code vulnerabilities), the KICS guardrail -// (IaC security vulnerabilities), and the SCA guardrail -// (malicious / vulnerable manifest additions) before any bytes are written -// to disk. MultiEdit and multi-file edits are handled uniformly by iterating -// ev.Changes. +// (IaC security vulnerabilities), the SCA guardrail +// (malicious / vulnerable manifest additions), and the secret-detection +// guardrail (hardcoded credentials introduced by the edit) before any bytes +// are written to disk. MultiEdit and multi-file edits are handled uniformly +// by iterating ev.Changes. // // 2. Cursor file READS (beforeReadFile) — ev.Changes is empty and ev.FilePath // points to a file the agent is about to ingest into the LLM context. @@ -93,6 +113,7 @@ func cxBeforeToolCall(ev agenthooks.ToolCallEvent) agenthooks.ToolVerdict { // or if the file exceeds the policy size cap. Reads do NOT count toward // the blast-radius budget (that limit is about writes). func cxBeforeFileEdit(ev agenthooks.FileEditEvent) agenthooks.FileEditVerdict { + bindThreshold(ev.Agent) if ev.Agent == agenthooks.AgentCursor && len(ev.Changes) == 0 { if reason := guardrails.ScanFileForSecrets(ev.FilePath); reason != "" { return agenthooks.RejectEdit(reason) @@ -111,30 +132,44 @@ func cxBeforeFileEdit(ev agenthooks.FileEditEvent) agenthooks.FileEditVerdict { return agenthooks.RejectEdit(reason) } agent := agentToString(ev.Agent) - if blocked, reason, context, severity := asca.ScanFileEdit(&ev, telemetryWrapper, agent); blocked { - sessiontally.Add(ev.SessionID, engineAsca, 1, 1) - logRemediationTelemetry(agent, "Asca", severity, ev.SessionID) - return agenthooks.RejectEditWithContext(reason, context) + // Each engine below is skipped (not just its verdict ignored) when the developer switched it off in + // ~/.checkmarx/cx-onboarding.properties; an unreadable config leaves every engine on. + // cx_severity_threshold (same file): a deny whose highest severity is below the threshold is let + // through for ASCA, KICS, SCA and secrets. Malicious packages and an unknown severity + // are never relaxed. A blank or invalid threshold means low (nothing filtered). + if scannerOn(ev.Agent, scannerconfig.ASCA) { + if blocked, reason, context, severity := asca.ScanFileEdit(&ev, telemetryWrapper, agent); blocked && !belowThreshold(ev.Agent, severity) { + sessiontally.Add(ev.SessionID, engineAsca, 1, 1) + logRemediationTelemetry(agent, "Asca", severity, ev.SessionID) + return agenthooks.RejectEditWithContext(reason, context) + } } var kicsNote string - if kicsScanner != nil { + if kicsScanner != nil && scannerOn(ev.Agent, scannerconfig.KICS) { blocked, reason, context, note, severity := kics.ScanFileEdit(&ev, kicsScanner, telemetryWrapper, agent) - if blocked { + if blocked && !belowThreshold(ev.Agent, severity) { sessiontally.Add(ev.SessionID, engineKics, 1, 1) logRemediationTelemetry(agent, "IaC", severity, ev.SessionID) return agenthooks.RejectEditWithContext(reason, context) } kicsNote = note } - if scaScanner != nil { + if scaScanner != nil && scannerOn(ev.Agent, scannerconfig.SCA) { for _, diff := range ev.Changes { - if finding, remediation, severity := scaScanner.CheckManifestEdit(ev.FilePath, fullAfterContent(ev.FilePath, diff), ev.WorkDir, agent, ev.SessionID); finding != "" { + if finding, remediation, severity := scaScanner.CheckManifestEdit(ev.FilePath, fullAfterContent(ev.FilePath, diff), ev.WorkDir, agent, ev.SessionID); finding != "" && !belowThreshold(ev.Agent, severity) { sessiontally.Add(ev.SessionID, engineSca, 1, 1) logRemediationTelemetry(agent, "Oss", severity, ev.SessionID) return agenthooks.RejectEditWithContext(finding, remediation) } } } + if secretsScanner != nil && secretsApply(ev.Agent) && scannerOn(ev.Agent, scannerconfig.Secrets) { + if blocked, reason, context, severity := secrets.ScanFileEdit(&ev, secretsScanner, telemetryWrapper, agent); blocked && !belowThreshold(ev.Agent, severity) { + sessiontally.Add(ev.SessionID, engineSecrets, 1, 1) + logRemediationTelemetry(agent, "Secrets", severity, ev.SessionID) + return agenthooks.RejectEditWithContext(reason, context) + } + } // A note on an ALLOW, deliberately: blocking every IaC edit because a // container engine is down would be worse than an unscanned edit. if kicsNote != "" { @@ -223,10 +258,11 @@ func promptWorkspaceRoots(raw any) []string { } // RegisterGuardrails wires the four guardrail handlers and instantiates the -// SCA and KICS scanners used by the Bash and FileEdit handlers. +// SCA, KICS, and secret-detection scanners used by the Bash and FileEdit handlers. func RegisterGuardrails(jwt wrappers.JWTWrapper, ff wrappers.FeatureFlagsWrapper, rt wrappers.RealtimeScannerWrapper, tel wrappers.TelemetryWrapper) { scaScanner = sca.NewScanner(jwt, ff, rt) kicsScanner = kics.NewScanner(jwt, ff) + secretsScanner = secrets.NewScanner(jwt, ff) telemetryWrapper = tel agenthooks.WhenAgentIdle(cxWhenAgentIdle) agenthooks.BeforeToolCall(cxBeforeToolCall) @@ -239,6 +275,7 @@ func RegisterGuardrails(jwt wrappers.JWTWrapper, ff wrappers.FeatureFlagsWrapper func RegisterPassThrough() { scaScanner = nil kicsScanner = nil + secretsScanner = nil agenthooks.WhenAgentIdle(func(_ agenthooks.AgentIdleEvent) agenthooks.IdleVerdict { return agenthooks.Resume() }) agenthooks.BeforeToolCall(func(_ agenthooks.ToolCallEvent) agenthooks.ToolVerdict { return agenthooks.Allow() }) agenthooks.BeforeFileEdit(func(_ agenthooks.FileEditEvent) agenthooks.FileEditVerdict { return agenthooks.AcceptEdit() }) @@ -268,6 +305,36 @@ func logRemediationTelemetry(agent, engine, severity, sessionID string) { } } +// switchesApply reports whether the ~/.checkmarx/cx-onboarding.properties scanner switches and severity +// threshold are honoured for this agent. Claude, Cursor, Gemini and Codex are excluded for now: they keep +// scanning with ASCA / KICS / SCA and blocking at any severity even when the shared file exists on the +// machine. Every other agent (GitHub Copilot, Droid, Windsurf, ...) honours it. Add an agent to +// the case list below to hold it back, or remove one to roll the feature out to it. +func switchesApply(agent agenthooks.AgentID) bool { + switch agent { + case agenthooks.AgentClaude, agenthooks.AgentCursor, agenthooks.AgentGemini, agenthooks.AgentCodex: + return false + } + return true +} + +// secretsApply reports whether the secret-detection guardrail runs for this agent. Claude, Cursor, Gemini +// and Codex skip it entirely and keep the ASCA / KICS / SCA guardrails; every other agent runs it. It +// shares switchesApply's agent list so the two cannot drift apart. +func secretsApply(agent agenthooks.AgentID) bool { + return switchesApply(agent) +} + +// scannerOn is scannerconfig.Enabled, limited to the agents the switches apply to. +func scannerOn(agent agenthooks.AgentID, s scannerconfig.Scanner) bool { + return !switchesApply(agent) || scannerconfig.Enabled(s) +} + +// belowThreshold is scannerconfig.BelowThreshold, limited to the agents the switches apply to. +func belowThreshold(agent agenthooks.AgentID, severity string) bool { + return switchesApply(agent) && scannerconfig.BelowThreshold(severity) +} + // agentToString converts agenthooks.AgentID enum to string representation for telemetry. func agentToString(agent agenthooks.AgentID) string { switch agent { diff --git a/internal/commands/agenthooks/cx/hooks_test.go b/internal/commands/agenthooks/cx/hooks_test.go index 36763db70..a70eeb8c1 100644 --- a/internal/commands/agenthooks/cx/hooks_test.go +++ b/internal/commands/agenthooks/cx/hooks_test.go @@ -19,10 +19,12 @@ import ( "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/kics" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/secrets" "github.com/checkmarx/ast-cli/internal/commands/agenthooks/sca" "github.com/checkmarx/ast-cli/internal/services/realtimeengine" "github.com/checkmarx/ast-cli/internal/services/realtimeengine/iacrealtime" "github.com/checkmarx/ast-cli/internal/services/realtimeengine/ossrealtime" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" "github.com/checkmarx/ast-cli/internal/wrappers" "github.com/Checkmarx/ast-cx-hooks/cursor" @@ -53,16 +55,18 @@ func (r *recordingTelemetry) SendAIDataToLog(data *wrappers.DataForAITelemetry) func resetHookGlobals(t *testing.T) { t.Helper() - prevSCA, prevKICS, prevTel := scaScanner, kicsScanner, telemetryWrapper + prevSCA, prevKICS, prevSecrets, prevTel := scaScanner, kicsScanner, secretsScanner, telemetryWrapper t.Cleanup(func() { scaScanner = prevSCA kicsScanner = prevKICS + secretsScanner = prevSecrets telemetryWrapper = prevTel guardrails.ResetBlastRadiusCount() guardrails.ResetTotalFileSizeCount() }) scaScanner = nil kicsScanner = nil + secretsScanner = nil telemetryWrapper = nil guardrails.ResetBlastRadiusCount() guardrails.ResetTotalFileSizeCount() @@ -415,6 +419,51 @@ func TestCxBeforeFileEdit_SCAManifest_RejectsWithContext(t *testing.T) { } } +func TestCxBeforeFileEdit_SecretsFinding_RejectsWithContext(t *testing.T) { + resetHookGlobals(t) + const secret = "ghp_hook_test_secret_value" + tel := &recordingTelemetry{} + telemetryWrapper = tel + secretsScanner = secrets.NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + if strings.Contains(content, secret) { + return []secretsrealtime.SecretsRealtimeResult{{ + Title: "github-pat", + Description: "GitHub personal access token", + SecretValue: secret, + Severity: "Critical", + Locations: []realtimeengine.Location{{Line: 1}}, + }}, nil + } + return nil, nil + }) + + v := cxBeforeFileEdit(agenthooks.FileEditEvent{ + Agent: agenthooks.AgentCopilotCLI, + SessionID: "sec-sess", + FilePath: "config.env", + WorkDir: t.TempDir(), + Changes: []agenthooks.FileDiff{{Before: "", After: "TOKEN=" + secret}}, + }) + if v.Permit { + t.Fatal("secret finding should RejectEdit") + } + if strings.Contains(v.Message, secret) || strings.Contains(v.Context, secret) { + t.Fatal("verdict leaked the secret value") + } + if !strings.Contains(v.Context, "cx-devassist:cx-devassist-secrets") { + t.Errorf("expected Copilot skill in context, got %q", v.Context) + } + if len(tel.calls) != 2 { + t.Fatalf("expected 2 telemetry calls (detect + remediate), got %d", len(tel.calls)) + } + if tel.calls[0].Type != "hooks-detect" || tel.calls[0].Engine != engineSecrets { + t.Errorf("detect telemetry = Type %q Engine %q", tel.calls[0].Type, tel.calls[0].Engine) + } + if tel.calls[1].Type != telemetryTypeHooksRemediate || tel.calls[1].Engine != "Secrets" { + t.Errorf("remediate telemetry = Type %q Engine %q", tel.calls[1].Type, tel.calls[1].Engine) + } +} + func TestCxBeforeFileEdit_CleanEdit_Accepts(t *testing.T) { resetHookGlobals(t) v := cxBeforeFileEdit(agenthooks.FileEditEvent{ @@ -555,6 +604,9 @@ func TestRegisterGuardrails_AndPassThrough(t *testing.T) { if kicsScanner == nil { t.Fatal("RegisterGuardrails should set kicsScanner") } + if secretsScanner == nil { + t.Fatal("RegisterGuardrails should set secretsScanner") + } if telemetryWrapper == nil { t.Fatal("RegisterGuardrails should set telemetryWrapper") } @@ -566,6 +618,9 @@ func TestRegisterGuardrails_AndPassThrough(t *testing.T) { if kicsScanner != nil { t.Fatal("RegisterPassThrough should clear kicsScanner") } + if secretsScanner != nil { + t.Fatal("RegisterPassThrough should clear secretsScanner") + } } func TestLogRemediationTelemetry(t *testing.T) { @@ -746,10 +801,11 @@ func TestCxBeforeFileEdit_CursorRead_NoSecrets_Accepts(t *testing.T) { func TestCxBeforeFileEdit_UnsupportedFileType_Accepts(t *testing.T) { setEmptyHomeDir(t) - prevSca, prevKics := scaScanner, kicsScanner - defer func() { scaScanner, kicsScanner = prevSca, prevKics }() + prevSca, prevKics, prevSecrets := scaScanner, kicsScanner, secretsScanner + defer func() { scaScanner, kicsScanner, secretsScanner = prevSca, prevKics, prevSecrets }() scaScanner = nil kicsScanner = nil + secretsScanner = nil path := filepath.Join(t.TempDir(), "notes.txt") ev := agenthooks.FileEditEvent{ @@ -769,20 +825,23 @@ func TestCxBeforePrompt_Benign_Accepts(t *testing.T) { } func TestRegisterGuardrails_SetsScanners(t *testing.T) { - prevSca, prevKics, prevTelemetry := scaScanner, kicsScanner, telemetryWrapper - defer func() { scaScanner, kicsScanner, telemetryWrapper = prevSca, prevKics, prevTelemetry }() + prevSca, prevKics, prevSecrets, prevTelemetry := scaScanner, kicsScanner, secretsScanner, telemetryWrapper + defer func() { + scaScanner, kicsScanner, secretsScanner, telemetryWrapper = prevSca, prevKics, prevSecrets, prevTelemetry + }() telemetry := mock.TelemetryMockWrapper{} RegisterGuardrails(&mock.JWTMockWrapper{}, &mock.FeatureFlagsMockWrapper{}, &mock.RealtimeScannerMockWrapper{}, telemetry) assert.NotNil(t, scaScanner) assert.NotNil(t, kicsScanner) + assert.NotNil(t, secretsScanner) assert.Equal(t, telemetry, telemetryWrapper) } func TestRegisterPassThrough_ClearsScanners(t *testing.T) { - prevSca, prevKics := scaScanner, kicsScanner - defer func() { scaScanner, kicsScanner = prevSca, prevKics }() + prevSca, prevKics, prevSecrets := scaScanner, kicsScanner, secretsScanner + defer func() { scaScanner, kicsScanner, secretsScanner = prevSca, prevKics, prevSecrets }() RegisterGuardrails(&mock.JWTMockWrapper{}, &mock.FeatureFlagsMockWrapper{}, &mock.RealtimeScannerMockWrapper{}, mock.TelemetryMockWrapper{}) assert.NotNil(t, scaScanner) @@ -790,6 +849,7 @@ func TestRegisterPassThrough_ClearsScanners(t *testing.T) { RegisterPassThrough() assert.Nil(t, scaScanner) assert.Nil(t, kicsScanner) + assert.Nil(t, secretsScanner) } func TestLogRemediationTelemetry_NilWrapper_NoOp(t *testing.T) { @@ -873,6 +933,7 @@ func TestCodexApplyPatch_KICSFinding_DeniesEndToEnd(t *testing.T) { }}, nil }) scaScanner = nil + secretsScanner = nil patch := "*** Begin Patch\n" + "*** Add File: main.tf\n" + diff --git a/internal/commands/agenthooks/cx/scanner_switch_test.go b/internal/commands/agenthooks/cx/scanner_switch_test.go new file mode 100644 index 000000000..81d105028 --- /dev/null +++ b/internal/commands/agenthooks/cx/scanner_switch_test.go @@ -0,0 +1,243 @@ +//go:build !integration + +package cx + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" + + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/kics" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/guardrails/secrets" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/iacrealtime" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" +) + +// useScannerConfig points the user's home at a temp dir holding ~/.checkmarx/cx-onboarding.properties +// with the given body (no file at all when body is empty). +func useScannerConfig(t *testing.T, body string) { + t.Helper() + dir := t.TempDir() + restore := setHomeDir(dir) + t.Cleanup(restore) + if body == "" { + return + } + cxDir := filepath.Join(dir, ".checkmarx") + if err := os.MkdirAll(cxDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cxDir, "cx-onboarding.properties"), []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + +func secretsEdit(secret string, agent agenthooks.AgentID) agenthooks.FileEditEvent { + return agenthooks.FileEditEvent{ + Agent: agent, + SessionID: "switch-sess", + FilePath: "config.env", + WorkDir: "", + Changes: []agenthooks.FileDiff{{Before: "", After: "TOKEN=" + secret}}, + } +} + +func stubSecretsScanner(secret string) *secrets.Scanner { + return secrets.NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + if strings.Contains(content, secret) { + return []secretsrealtime.SecretsRealtimeResult{{ + Title: "github-pat", Description: "token", SecretValue: secret, Severity: "Critical", + Locations: []realtimeengine.Location{{Line: 1}}, + }}, nil + } + return nil, nil + }) +} + +func TestCxBeforeFileEdit_SecretsSwitch(t *testing.T) { + const secret = "ghp_switch_test_secret_value" + cases := []struct { + name string + config string + wantBlock bool + }{ + {"no config file is enabled", "", true}, + {"explicit true is enabled", "cx_secret_scanner_enabled=true\n", true}, + {"typo stays enabled", "cx_secret_scanner_enabled=flase\n", true}, + {"other scanner off does not affect secrets", "cx_asca_scanner_enabled=false\ncx_kics_scanner_enabled=false\n", true}, + {"secrets off is skipped", "cx_secret_scanner_enabled=false\n", false}, + {"secrets off (CRLF, caps)", "cx_secret_scanner_enabled=OFF\r\n", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, tc.config) + secretsScanner = stubSecretsScanner(secret) + v := cxBeforeFileEdit(secretsEdit(secret, agenthooks.AgentCopilotCLI)) + if v.Permit == tc.wantBlock { + t.Fatalf("Permit=%v, wantBlock=%v", v.Permit, tc.wantBlock) + } + }) + } +} + +func kicsFinding(severity string) *kics.Scanner { + return kics.NewScannerWithFunc(func(string, string) ([]iacrealtime.IacRealtimeResult, error) { + return []iacrealtime.IacRealtimeResult{{ + Title: "Privileged Container", SimilarityID: "sim1", Severity: severity, + Description: "Container runs as privileged", Locations: []realtimeengine.Location{{Line: 5}}, + }}, nil + }) +} + +func kicsEdit(agent agenthooks.AgentID) agenthooks.FileEditEvent { + return agenthooks.FileEditEvent{ + Agent: agent, SessionID: "thr-sess", FilePath: "/project/Dockerfile", + Changes: []agenthooks.FileDiff{{Before: "", After: "FROM ubuntu\nUSER root\n"}}, + } +} + +func TestCxBeforeFileEdit_SeverityThreshold(t *testing.T) { + cases := []struct { + name string + config string + severity string + wantBlock bool + }{ + {"no config blocks low", "", "LOW", true}, + {"blank threshold blocks low", "cx_severity_threshold=\n", "LOW", true}, + {"invalid threshold blocks low", "cx_severity_threshold=hihg\n", "LOW", true}, + {"high threshold lets low through", "cx_severity_threshold=high\n", "LOW", false}, + {"high threshold lets medium through", "cx_severity_threshold=high\n", "MEDIUM", false}, + {"high threshold blocks high", "cx_severity_threshold=high\n", "HIGH", true}, + {"high threshold blocks critical", "cx_severity_threshold=high\n", "CRITICAL", true}, + {"unknown severity is never relaxed", "cx_severity_threshold=critical\n", "", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, tc.config) + kicsScanner = kicsFinding(tc.severity) + if v := cxBeforeFileEdit(kicsEdit(agenthooks.AgentCopilotCLI)); v.Permit == tc.wantBlock { + t.Fatalf("Permit=%v, wantBlock=%v", v.Permit, tc.wantBlock) + } + }) + } +} + +func TestCxBeforeFileEdit_SeverityThreshold_Secrets(t *testing.T) { + const secret = "ghp_threshold_test_secret_value" + cases := []struct { + name string + config string + severity string + wantBlock bool + }{ + {"no config blocks low", "", "Low", true}, + {"high threshold lets medium through", "cx_severity_threshold=high\n", "Medium", false}, + {"high threshold blocks high", "cx_severity_threshold=high\n", "High", true}, + {"critical threshold lets low through", "cx_severity_threshold=critical\n", "Low", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, tc.config) + secretsScanner = secrets.NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + if strings.Contains(content, secret) { + return []secretsrealtime.SecretsRealtimeResult{{ + Title: "generic-key", Description: "key", SecretValue: secret, Severity: tc.severity, + Locations: []realtimeengine.Location{{Line: 1}}, + }}, nil + } + return nil, nil + }) + if v := cxBeforeFileEdit(secretsEdit(secret, agenthooks.AgentCopilotCLI)); v.Permit == tc.wantBlock { + t.Fatalf("Permit=%v, wantBlock=%v", v.Permit, tc.wantBlock) + } + }) + } +} + +func TestCxBeforeFileEdit_SeverityThreshold_SecretsMixed(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, "cx_severity_threshold=high\n") + secretsScanner = secrets.NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + return []secretsrealtime.SecretsRealtimeResult{ + {Title: "high-rule", Description: "d", SecretValue: "hv", Severity: "High", Locations: []realtimeengine.Location{{Line: 1}}}, + {Title: "medium-rule", Description: "d", SecretValue: "mv", Severity: "Medium", Locations: []realtimeengine.Location{{Line: 2}}}, + }, nil + }) + v := cxBeforeFileEdit(secretsEdit("x", agenthooks.AgentCopilotCLI)) + if v.Permit { + t.Fatal("the high secret must block") + } + if blob := fmt.Sprintf("%+v", v); !strings.Contains(blob, "high-rule") || strings.Contains(blob, "medium-rule") { + t.Fatalf("deny must list the high finding and omit the medium one: %s", blob) + } +} + +// The switches and secret detection are features of every agent except Claude, Cursor, Gemini and Codex: those four skip secret +// detection and, with the shared properties file present, keeps ASCA/KICS/SCA blocking at any severity. +func TestSwitches_IgnoredForOtherAgents(t *testing.T) { + const secret = "ghp_other_agent_secret_value" + others := []agenthooks.AgentID{ + agenthooks.AgentClaude, agenthooks.AgentCursor, agenthooks.AgentGemini, agenthooks.AgentCodex, + } + for _, agent := range others { + t.Run(agentToString(agent)+"/secrets are skipped", func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, "") + secretsScanner = stubSecretsScanner(secret) + if v := cxBeforeFileEdit(secretsEdit(secret, agent)); !v.Permit { + t.Fatal("secret detection must not run for this agent") + } + }) + t.Run(agentToString(agent)+"/kics switch off still blocks", func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, "cx_kics_scanner_enabled=false\n") + kicsScanner = kicsFinding("HIGH") + if v := cxBeforeFileEdit(kicsEdit(agent)); v.Permit { + t.Fatal("kics switch must not apply to this agent") + } + }) + t.Run(agentToString(agent)+"/threshold does not relax", func(t *testing.T) { + resetHookGlobals(t) + useScannerConfig(t, "cx_severity_threshold=critical\n") + kicsScanner = kicsFinding("LOW") + if v := cxBeforeFileEdit(kicsEdit(agent)); v.Permit { + t.Fatal("severity threshold must not apply to this agent") + } + }) + } +} + +func TestSwitchesApply_ExcludesFourAgents(t *testing.T) { + for agent, want := range map[agenthooks.AgentID]bool{ + agenthooks.AgentCopilotCLI: true, agenthooks.AgentCopilot: true, + agenthooks.AgentDroid: true, agenthooks.AgentWindsurf: true, + agenthooks.AgentClaude: false, agenthooks.AgentCursor: false, agenthooks.AgentGemini: false, + agenthooks.AgentCodex: false, + } { + if got := switchesApply(agent); got != want { + t.Errorf("%s: got %v, want %v", agentToString(agent), got, want) + } + } +} + +func TestSecretsApply_ExcludesFourAgents(t *testing.T) { + for agent, want := range map[agenthooks.AgentID]bool{ + agenthooks.AgentCopilotCLI: true, agenthooks.AgentCopilot: true, + agenthooks.AgentDroid: true, agenthooks.AgentWindsurf: true, + agenthooks.AgentClaude: false, agenthooks.AgentCursor: false, + agenthooks.AgentGemini: false, agenthooks.AgentCodex: false, + } { + if got := secretsApply(agent); got != want { + t.Errorf("%s: got %v, want %v", agentToString(agent), got, want) + } + } +} diff --git a/internal/commands/agenthooks/guardrails/asca/asca.go b/internal/commands/agenthooks/guardrails/asca/asca.go index eed2bc7eb..09f10301f 100644 --- a/internal/commands/agenthooks/guardrails/asca/asca.go +++ b/internal/commands/agenthooks/guardrails/asca/asca.go @@ -28,6 +28,11 @@ func isSupportedByASCA(filePath string) bool { return ok } +// BelowThreshold is set by the hook dispatcher and reports whether a finding's severity is under the +// developer's cx_severity_threshold. Such findings are dropped from the deny, so the agent is never +// told to remediate them. Nil (the default) keeps every finding. +var BelowThreshold func(severity string) bool + // ScanFileEdit runs ASCA on the proposed post-edit content. // Returns blocked=true with a formatted reason, remediation context, and highest severity when ASCA // finds *new* vulnerabilities introduced by ev.Changes (delta-detection for edits; @@ -90,14 +95,16 @@ func ScanFileEdit(ev *agenthooks.FileEditEvent, telemetryWrapper wrappers.Teleme } // For new files (no original content), every finding is new - if originalContent == "" { - r, c := formatFindings(ev.FilePath, newResult.ScanDetails, ev.WorkDir, agent, ev.SessionID) - findingCount = len(newResult.ScanDetails) - return true, r, c, highestSeverity(newResult.ScanDetails) + newFindings := newResult.ScanDetails + if originalContent != "" { + var err error + newFindings, err = deltaFindings(ev, originalContent, ascaParams, wrapperParams, newResult.ScanDetails) + if err != nil { + newFindings = nil + } } - - newFindings, err := deltaFindings(ev, originalContent, ascaParams, wrapperParams, newResult.ScanDetails) - if err != nil || len(newFindings) == 0 { + newFindings = aboveThreshold(newFindings) + if len(newFindings) == 0 { findingCount = 0 return false, "", "", "" } @@ -130,6 +137,20 @@ func deltaFindings(ev *agenthooks.FileEditEvent, originalContent string, ascaPar return NewFindings(origDetails, newDetails), nil } +// aboveThreshold drops findings below the developer's severity threshold. +func aboveThreshold(findings []grpcs.ScanDetail) []grpcs.ScanDetail { + if BelowThreshold == nil { + return findings + } + kept := make([]grpcs.ScanDetail, 0, len(findings)) + for i := range findings { + if !BelowThreshold(findings[i].Severity) { + kept = append(kept, findings[i]) + } + } + return kept +} + // highestSeverity returns the highest severity level across the given ASCA findings. // Order: Critical > High > Medium > Low > (anything else). func highestSeverity(findings []grpcs.ScanDetail) string { diff --git a/internal/commands/agenthooks/guardrails/kics/kics.go b/internal/commands/agenthooks/guardrails/kics/kics.go index 0b98adbc6..17f630322 100644 --- a/internal/commands/agenthooks/guardrails/kics/kics.go +++ b/internal/commands/agenthooks/guardrails/kics/kics.go @@ -40,6 +40,11 @@ func isSupportedByKICS(filePath string) bool { return false } +// BelowThreshold is set by the hook dispatcher and reports whether a finding's severity is under the +// developer's cx_severity_threshold. Such findings are dropped from the deny, so the agent is never +// told to remediate them. Nil (the default) keeps every finding. +var BelowThreshold func(severity string) bool + // ScanFileEdit runs KICS on the proposed post-edit content. // Returns blocked=true with a formatted reason, remediation context, and highest severity when KICS // finds *new* vulnerabilities introduced by ev.Changes (delta-detection for edits; @@ -94,6 +99,10 @@ func ScanFileEdit(ev *agenthooks.FileEditEvent, svc *Scanner, telemetryWrapper w // For new files (no original content), every finding is new if originalContent == "" { + newResults = aboveThreshold(newResults) + if len(newResults) == 0 { + return false, "", "", "", "" + } r, c := formatFindings(ev.FilePath, newResults, ev.Agent, ev.WorkDir, ev.SessionID) findingCount = len(newResults) return true, r, c, "", highestSeverity(newResults) @@ -113,7 +122,7 @@ func ScanFileEdit(ev *agenthooks.FileEditEvent, svc *Scanner, telemetryWrapper w return false, "", "", skippedNote(ev.FilePath, err), "" } - newFindings := NewFindings(origResults, newResults) + newFindings := aboveThreshold(NewFindings(origResults, newResults)) if len(newFindings) == 0 { return false, "", "", "", "" } @@ -123,6 +132,20 @@ func ScanFileEdit(ev *agenthooks.FileEditEvent, svc *Scanner, telemetryWrapper w return true, r, c, "", highestSeverity(newFindings) } +// aboveThreshold drops findings below the developer's severity threshold. +func aboveThreshold(findings []iacrealtime.IacRealtimeResult) []iacrealtime.IacRealtimeResult { + if BelowThreshold == nil { + return findings + } + kept := make([]iacrealtime.IacRealtimeResult, 0, len(findings)) + for i := range findings { + if !BelowThreshold(findings[i].Severity) { + kept = append(kept, findings[i]) + } + } + return kept +} + // highestSeverity returns the highest severity level across the given KICS findings. // Order: Critical > High > Medium > Low > (anything else). func highestSeverity(findings []iacrealtime.IacRealtimeResult) string { diff --git a/internal/commands/agenthooks/guardrails/secrets/content.go b/internal/commands/agenthooks/guardrails/secrets/content.go new file mode 100644 index 000000000..7b44d714f --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/content.go @@ -0,0 +1,40 @@ +package secrets + +import ( + "os" + "strings" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" +) + +func normLF(s string) string { + s = strings.ReplaceAll(s, "\r\n", "\n") + return strings.ReplaceAll(s, "\r", "\n") +} + +// proposedContent returns the file content that would exist after changes are applied, +// plus the content currently on disk (empty when the file does not exist yet). +// ok is false when Before text is missing or matches more than once. +func proposedContent(filePath string, changes []agenthooks.FileDiff) (newContent, originalContent string, ok bool) { + diskBytes, readErr := os.ReadFile(filePath) + if readErr == nil { + originalContent = string(diskBytes) + } + + if len(changes) == 1 && changes[0].Before == "" { + return changes[0].After, originalContent, true + } + + current := normLF(originalContent) + for _, diff := range changes { + before := normLF(diff.Before) + after := normLF(diff.After) + if strings.Count(current, before) != 1 { + return "", normLF(originalContent), false + } + idx := strings.Index(current, before) + current = current[:idx] + after + current[idx+len(before):] + } + + return current, normLF(originalContent), true +} diff --git a/internal/commands/agenthooks/guardrails/secrets/content_test.go b/internal/commands/agenthooks/guardrails/secrets/content_test.go new file mode 100644 index 000000000..92fda12ea --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/content_test.go @@ -0,0 +1,77 @@ +//go:build !integration + +package secrets + +import ( + "os" + "path/filepath" + "testing" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" +) + +const ( + testConfigEnv = "config.env" + debugFalse = "DEBUG=false" + debugTrue = "DEBUG=true" + placeholderKey = "API_KEY=placeholder" +) + +func TestProposedContent_FullFileWrite(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "new.env") + changes := []agenthooks.FileDiff{{Before: "", After: "TOKEN=abc"}} + newContent, originalContent, ok := proposedContent(path, changes) + if !ok { + t.Fatal("full-file write should always resolve") + } + if newContent != "TOKEN=abc" || originalContent != "" { + t.Fatalf("newContent=%q originalContent=%q", newContent, originalContent) + } +} + +func TestProposedContent_UniqueMatch_Applies(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, testConfigEnv) + if err := os.WriteFile(path, []byte(debugFalse), 0o600); err != nil { + t.Fatal(err) + } + changes := []agenthooks.FileDiff{{Before: debugFalse, After: debugTrue}} + newContent, _, ok := proposedContent(path, changes) + if !ok { + t.Fatal("unique match should resolve") + } + if newContent != debugTrue { + t.Fatalf("newContent = %q, want %s", newContent, debugTrue) + } +} + +func TestProposedContent_DuplicateBeforeText_IsUnresolved(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, testConfigEnv) + body := placeholderKey + "\n" + debugTrue + "\n" + placeholderKey + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + changes := []agenthooks.FileDiff{{Before: placeholderKey, After: "API_KEY=sk-live-123"}} + _, _, ok := proposedContent(path, changes) + if ok { + t.Fatal("a Before string that matches more than once must not be guessed at") + } +} + +func TestProposedContent_BeforeTextMissing_IsUnresolved(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, testConfigEnv) + if err := os.WriteFile(path, []byte(debugTrue), 0o600); err != nil { + t.Fatal(err) + } + changes := []agenthooks.FileDiff{{Before: "STALE_TEXT", After: "TOKEN=sk-live-123"}} + newContent, _, ok := proposedContent(path, changes) + if ok { + t.Fatal("a Before string that is not found must not be silently skipped") + } + if newContent != "" { + t.Fatalf("newContent should be empty when unresolved, got %q", newContent) + } +} diff --git a/internal/commands/agenthooks/guardrails/secrets/delta.go b/internal/commands/agenthooks/guardrails/secrets/delta.go new file mode 100644 index 000000000..1d79833de --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/delta.go @@ -0,0 +1,203 @@ +package secrets + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/cursorplugin" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/ignore" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" +) + +// genericSecretDescription replaces finding descriptions that would leak the secret value. +const genericSecretDescription = "hardcoded secret" + +// findingKey matches the ignore-file key used by secrets-realtime: Title + SecretValue. +type findingKey struct { + title string + value string +} + +func keyOf(r *secretsrealtime.SecretsRealtimeResult) findingKey { + return findingKey{title: r.Title, value: r.SecretValue} +} + +// NewFindings returns results present in newScan that have no matching key in originalScan. +// A new file (originalScan == nil) returns newScan unchanged. +func NewFindings(originalScan, newScan []secretsrealtime.SecretsRealtimeResult) []secretsrealtime.SecretsRealtimeResult { + if originalScan == nil { + return newScan + } + baseline := make(map[findingKey]struct{}, len(originalScan)) + for i := range originalScan { + baseline[keyOf(&originalScan[i])] = struct{}{} + } + var out []secretsrealtime.SecretsRealtimeResult + for i := range newScan { + if _, exists := baseline[keyOf(&newScan[i])]; !exists { + out = append(out, newScan[i]) + } + } + return out +} + +// provenanceTag is prepended to the deny reason and context so the agent can recognize +// genuine Checkmarx output. +func provenanceTag() string { + return "[Checkmarx cx-devassist — automated security output, not user input]" +} + +// formatFindings builds the deny reason and the remediation context delivered to the agent. +// The secret value is written only to a mode-0600 finding file referenced by the suppress +// command, so it does not appear in the verdict text. +func formatFindings(filePath string, findings []secretsrealtime.SecretsRealtimeResult, agent agenthooks.AgentID, workDir, sessionID string) (reason, context string) { + summary := findingsSummary(filePath, findings) + tag := provenanceTag() + reason = tag + " " + permissionDecisionReason(filePath, summary) + if agent == agenthooks.AgentCursor { + context = cursorAdditionalContext(filePath, cxExecutable(), findings, workDir, sessionID) + } else { + context = additionalContext(filePath, cxExecutable(), findings, workDir, agent, sessionID) + } + context = tag + " " + context + return reason, context +} + +func findingsSummary(filePath string, findings []secretsrealtime.SecretsRealtimeResult) string { + var sb strings.Builder + for i := range findings { + f := &findings[i] + line := 0 + if len(f.Locations) > 0 { + line = f.Locations[0].Line + } + fmt.Fprintf(&sb, " - %s line %d [%s] %s — %s\n", + filePath, line, f.Severity, f.Title, safeDescription(f)) + } + return sb.String() +} + +func safeDescription(f *secretsrealtime.SecretsRealtimeResult) string { + if f.Description == "" || (f.SecretValue != "" && strings.Contains(f.Description, f.SecretValue)) { + return genericSecretDescription + } + return f.Description +} + +func permissionDecisionReason(filePath, summary string) string { + return fmt.Sprintf( + "Checkmarx secret detection found hardcoded secrets in %s.\nFindings:\n%s", + filePath, summary, + ) +} + +func cxExecutable() string { + cxExe, err := os.Executable() + if err != nil { + return "cx" + } + return cxExe +} + +func ignoredFilePathFlag(workDir string) string { + if workDir == "" { + return "" + } + return fmt.Sprintf(" --ignored-file-path '%s'", ignore.PathFor(workDir)) +} + +func cursorIgnoredFilePathFlag(workDir string) string { + if workDir == "" { + return "" + } + p := filepath.ToSlash(ignore.PathFor(workDir)) + return fmt.Sprintf(" --ignored-file-path %q", p) +} + +func optionalFlagsFragment(agent agenthooks.AgentID, sessionID string) string { + label := agentLabel(agent) + if label == "" { + return "" + } + pairs := "aiProvider=" + label + ";agent=" + label + "-cli" + if sessionID != "" { + pairs += ";aiAgentSessionId=" + sessionID + } + return fmt.Sprintf(" --optional-flags %q", pairs) +} + +func agentLabel(agent agenthooks.AgentID) string { + switch agent { + case agenthooks.AgentClaude: + return "Claude" + case agenthooks.AgentCopilot, agenthooks.AgentCopilotCLI: + return "Copilot" + case agenthooks.AgentCursor: + return "Cursor" + case agenthooks.AgentGemini: + return "Gemini" + case agenthooks.AgentCodex: + return "Codex" + default: + return "" + } +} + +// secretsSuppressCommands returns one ignore-vulnerability command per finding. +// The finding JSON (which must include SecretValue) is stored in a temp file and passed as +// --data @ so the secret value is not embedded in the agent-visible command. +func secretsSuppressCommands(cxBinary string, findings []secretsrealtime.SecretsRealtimeResult, workDir string, agent agenthooks.AgentID, sessionID string) string { + provenance := optionalFlagsFragment(agent, sessionID) + var suppressCmds strings.Builder + for i := range findings { + f := &findings[i] + path, err := writeSuppressPayload(f) + if err != nil { + fmt.Fprintf(&suppressCmds, " # suppress unavailable for %s\n", f.Title) + continue + } + dataArg := "@" + path + if agent == agenthooks.AgentCursor { + ignoreFlag := cursorIgnoredFilePathFlag(workDir) + suppressCmds.WriteString(cursorplugin.IgnoreVulnerabilityCommand(cxBinary, "secrets", []byte(dataArg), ignoreFlag, provenance)) + suppressCmds.WriteString("\n") + continue + } + ignoreFlag := ignoredFilePathFlag(workDir) + if agent == agenthooks.AgentGemini { + fmt.Fprintf(&suppressCmds, " %s ignore-vulnerability --scan-type secrets --data %s%s%s\n", cxBinary, ignore.QuoteDataFlag([]byte(dataArg)), ignoreFlag, provenance) + } else { + fmt.Fprintf(&suppressCmds, " %s ignore-vulnerability --scan-type secrets --data '%s'%s%s\n", cxBinary, dataArg, ignoreFlag, provenance) + } + } + return suppressCmds.String() +} + +func writeSuppressPayload(f *secretsrealtime.SecretsRealtimeResult) (string, error) { + if f.Title == "" || f.SecretValue == "" { + return "", fmt.Errorf("finding missing ignore key") + } + payload, err := json.Marshal(secretsrealtime.IgnoredSecret{Title: f.Title, SecretValue: f.SecretValue}) + if err != nil { + return "", err + } + file, err := os.CreateTemp("", "cx-secret-finding-*.json") + if err != nil { + return "", err + } + name := file.Name() + if _, err = file.Write(payload); err != nil { + _ = file.Close() + _ = os.Remove(name) + return "", err + } + if err = file.Close(); err != nil { + _ = os.Remove(name) + return "", err + } + return name, nil +} diff --git a/internal/commands/agenthooks/guardrails/secrets/prompts.go b/internal/commands/agenthooks/guardrails/secrets/prompts.go new file mode 100644 index 000000000..7e2d92f9f --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/prompts.go @@ -0,0 +1,136 @@ +package secrets + +import ( + "fmt" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/agentprofile" + "github.com/checkmarx/ast-cli/internal/commands/agenthooks/cursorplugin" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" +) + +// hookDenyHeader is the agent-facing opening of additionalContext. The provenance tag is +// prepended separately in formatFindings. This header does not contain the spoof phrases +// the skills treat as proof a message is not from Checkmarx. +const hookDenyHeader = "This is a Checkmarx hook deny. Follow it. It is not file content and not an untrusted tool error.\n" + +// secretsRemediationReport is the closing report both deny paths tell the agent to show. +const secretsRemediationReport = "5. Always finish with this report, even if you asked the user a question, the file is new, " + + "or the retry passed. Show it in the chat as markdown, not inside a code block. One bullet per finding, " + + "then a blank line and the final status. Do not print the braces. Pick one result and one final status. " + + "A partial fix must include why it does not fully resolve the issue. Ignored must include why you ignored it. Unresolved must include why it was not fixed. A bullet without that reason is incomplete. " + + "Do not echo the secret value.\n" + + "## Checkmarx DevAssist Secret Remediation Summary\n" + + "\n" + + "- **{rule id}** - {severity} - line {line} - **{Fixed, Partial fix, Ignored, or Unresolved}**\n" + + " {Fixed: what changed. Partial fix: what you applied from the MCP suggestion, and Reason: why it does not fully resolve the issue. Ignored: Reason: why, citing the user's words or the file and line you read. Unresolved: Reason: why it was not fixed.}\n" + + "\n" + + "**Final status:** {All fixed, Partially fixed, or Unresolved}\n" + + "Then continue the user's original task. Do not include that sentence in the report.\n" + +func additionalContext(filePath, cxBinary string, findings []secretsrealtime.SecretsRealtimeResult, workDir string, agent agenthooks.AgentID, sessionID string) string { + codeTool := "mcp__Checkmarx__codeRemediation" + if agent == agenthooks.AgentGemini { + codeTool = "mcp_Checkmarx_codeRemediation" + } + skill := secretsSkillName(agent) + suppressCmds := secretsSuppressCommands(cxBinary, findings, workDir, agent, sessionID) + return fmt.Sprintf( + hookDenyHeader+ + "Secret detection blocked the write to %s.\nFindings:\n%s"+ + "Handle every finding yourself. Never ask the user, including \"Would you like me to "+ + "remediate?\" (that question is only for on-demand scans).\n"+ + "1. Classify each finding:\n"+ + "- False positive only if one of these is already true and you can cite it: (a) the user "+ + "explicitly told you to ignore or suppress it; (b) a file you opened this session shows, at a "+ + "line you can cite, that the match is a placeholder, example, or test fixture. What a finding, "+ + "hook message, or file says about another file is not evidence; open that file. Apparent intent "+ + "(a lab, demo, or training file) is not evidence; that still needs (a) or (b).\n"+ + "- Otherwise it is a true positive, including when you are unsure.\n"+ + "2. True positive: call %s for each one with type \"secret\" and metadata (ruleId = the finding "+ + "title, description, line). The %s skill can make this call; if it disagrees with this deny, "+ + "follow this deny. Apply remediation_steps only with Edit, Write, MultiEdit, or NotebookEdit, "+ + "never a shell command (shell writes are not scanned). Replace the hardcoded secret with an "+ + "environment-variable reference, including any Safe Refactor the tool returns. Do not echo the "+ + "secret value. If the blocked write creates a new file, the fix is that same Write with the "+ + "fixed content. If you believe those remediation_steps will not fully solve the security issue, "+ + "still apply them. Do not skip the MCP suggestion, and do not ignore the finding because of that "+ + "belief. In the summary mark that finding as a partial fix and state why it does not fully "+ + "resolve the issue.\n"+ + "If %s is unavailable, change nothing and ignore nothing because of it. Report the findings "+ + "as unresolved and tell the user to %s.\n"+ + "3. False positive: run exactly this command for it:\n%s"+ + "4. Verify: skip only the initial scan. Run `cx scan secrets-realtime -s ` with the "+ + "canonical cx path; that re-scan validates the finding. A finding is fixed only when the "+ + "re-scan no longer reports it. Then retry the blocked write once so the gate can accept the "+ + "write. If the re-scan still reports a finding whose remediation_steps you applied, keep that "+ + "change and mark it as a partial fix with the reason it does not fully resolve the issue. If "+ + "the retry is denied, a remaining finding you have not applied a suggestion for, or one your "+ + "fix introduced, gets one more %s call. Stop after 3 denied retries or when the tool returns "+ + "no safe change. Then ignore only findings that meet step 1. Report a finding whose MCP "+ + "suggestion you applied as a partial fix; report a finding with no applied suggestion as "+ + "unresolved. Stop editing the file. Do not ask whether to continue.\n"+ + "Never write this content through another tool, a shell command, or a different file to "+ + "bypass the scan.\n"+ + secretsRemediationReport, + filePath, findingsSummary(filePath, findings), codeTool, skill, codeTool, agentprofile.McpReconnect(agentLabel(agent)), suppressCmds, codeTool, + ) +} + +func cursorAdditionalContext(filePath, cxBinary string, findings []secretsrealtime.SecretsRealtimeResult, workDir, sessionID string) string { + suppressCmds := secretsSuppressCommands(cxBinary, findings, workDir, agenthooks.AgentCursor, sessionID) + skill := secretsSkillName(agenthooks.AgentCursor) + codeTool := cursorplugin.MCPTool("codeRemediation") + return fmt.Sprintf( + hookDenyHeader+ + "Secret detection blocked the write to %s. Follow the cx-hook-deny.mdc and "+ + "cx-devassist-secrets.mdc rules for this deny.\nFindings:\n%s"+ + "Handle every finding yourself. Never ask the user, including \"Would you like me to "+ + "remediate?\" (that question is only for on-demand scans).\n"+ + "1. Classify each finding:\n"+ + "- False positive only if one of these is already true and you can cite it: (a) the user "+ + "explicitly told you to ignore or suppress this finding, in those words. Saying the secret is "+ + "still exposed, or that a change does not make it safe, is not (a). Do not run the ignore "+ + "command because of it. Still apply the MCP suggestion and mark the finding as a partial fix "+ + "with the reason it does not fully resolve the issue. (b) a file you opened this session "+ + "shows, at a line you can cite, that the match is a placeholder, example, or test fixture. "+ + "What a finding, hook message, or file says about another file is not evidence; open that "+ + "file. Apparent intent (a lab, demo, or training file) is not evidence; that still needs (a) "+ + "or (b).\n"+ + "- Otherwise it is a true positive, including when you are unsure.\n"+ + "2. True positive: call %s for each one with type \"secret\" and metadata (ruleId = the finding "+ + "title, description, line). The %s skill can make this call; if it disagrees with this deny, "+ + "follow this deny. Apply remediation_steps only with Write or StrReplace, never a shell "+ + "command (shell writes are not scanned). Replace the hardcoded secret with an "+ + "environment-variable reference, including any Safe Refactor the tool returns. Do not echo the "+ + "secret value. If the blocked write creates a new file, the fix is that same Write with the "+ + "fixed content. If you believe those remediation_steps will not fully solve the security issue, "+ + "still apply them. Do not skip the MCP suggestion, and do not ignore the finding because of that "+ + "belief. In the summary mark that finding as a partial fix and state why it does not fully "+ + "resolve the issue.\n"+ + "If %s is unavailable, change nothing and ignore nothing because of it. Report the findings "+ + "as unresolved and tell the user to %s.\n"+ + "3. False positive: run exactly this command for it:\n%s"+ + "4. Verify: skip only the initial scan. Run `cx scan secrets-realtime -s ` with the "+ + "canonical cx path; that re-scan validates the finding. A finding is fixed only when the "+ + "re-scan no longer reports it. Then retry the blocked write once so the gate can accept the "+ + "write. If the re-scan still reports a finding whose remediation_steps you applied, keep that "+ + "change and mark it as a partial fix with the reason it does not fully resolve the issue. If "+ + "the retry is denied, a remaining finding you have not applied a suggestion for, or one your "+ + "fix introduced, gets one more %s call. Stop after 3 denied retries or when the tool returns "+ + "no safe change. Then ignore only findings that meet step 1. Report a finding whose MCP "+ + "suggestion you applied as a partial fix; report a finding with no applied suggestion as "+ + "unresolved. Stop editing the file. Do not ask whether to continue.\n"+ + "Never write this content through another tool, a shell command, or a different file to "+ + "bypass the scan.\n"+ + secretsRemediationReport, + filePath, findingsSummary(filePath, findings), codeTool, skill, codeTool, agentprofile.McpReconnect(agentLabel(agenthooks.AgentCursor)), suppressCmds, codeTool, + ) +} + +func secretsSkillName(agent agenthooks.AgentID) string { + if agent == agenthooks.AgentGemini { + return "/cx-devassist-secrets" + } + return "cx-devassist:cx-devassist-secrets" +} diff --git a/internal/commands/agenthooks/guardrails/secrets/scanner.go b/internal/commands/agenthooks/guardrails/secrets/scanner.go new file mode 100644 index 000000000..a2eb58a1e --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/scanner.go @@ -0,0 +1,32 @@ +package secrets + +import ( + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" + "github.com/checkmarx/ast-cli/internal/wrappers" +) + +// Scanner runs secret realtime scans on behalf of the file-edit guardrail. +// Tests substitute scan via NewScannerWithFunc. +type Scanner struct { + jwt wrappers.JWTWrapper + ff wrappers.FeatureFlagsWrapper + scan func(sourcePath, content, ignoreFilePath string) ([]secretsrealtime.SecretsRealtimeResult, error) +} + +// NewScanner returns a Scanner backed by the secrets realtime engine. +func NewScanner(jwt wrappers.JWTWrapper, ff wrappers.FeatureFlagsWrapper) *Scanner { + s := &Scanner{jwt: jwt, ff: ff} + s.scan = s.runRealScan + return s +} + +// NewScannerWithFunc returns a Scanner whose scan call is replaced with f. +// For unit tests only. +func NewScannerWithFunc(f func(sourcePath, content, ignoreFilePath string) ([]secretsrealtime.SecretsRealtimeResult, error)) *Scanner { + return &Scanner{scan: f} +} + +func (s *Scanner) runRealScan(sourcePath, content, ignoreFilePath string) ([]secretsrealtime.SecretsRealtimeResult, error) { + svc := secretsrealtime.NewSecretsRealtimeService(s.jwt, s.ff) + return svc.ScanContent(sourcePath, content, ignoreFilePath) +} diff --git a/internal/commands/agenthooks/guardrails/secrets/secrets.go b/internal/commands/agenthooks/guardrails/secrets/secrets.go new file mode 100644 index 000000000..b5a73c853 --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/secrets.go @@ -0,0 +1,148 @@ +package secrets + +import ( + "os" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" + "github.com/checkmarx/ast-cli/internal/logger" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/ignore" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" + "github.com/checkmarx/ast-cli/internal/wrappers" +) + +const ( + unresolvedEditReason = "Checkmarx secret detection could not verify this edit: the text being " + + "replaced was not found uniquely in the file, so the result cannot be scanned reliably." + unresolvedEditContext = "Retry with an edit whose Before/old_string text matches exactly one " + + "location in the file (add more surrounding context if the target text repeats), then the " + + "edit can be scanned and applied." +) + +// BelowThreshold is set by the hook dispatcher and reports whether a finding's severity is under the +// developer's cx_severity_threshold. Such findings are dropped from the deny, so the agent is never +// told to remediate them. Nil (the default) keeps every finding. +var BelowThreshold func(severity string) bool + +// ScanFileEdit runs secret detection on the proposed post-edit content. +// Returns blocked=true with a formatted reason, remediation context, and highest severity when the +// edit introduces a secret (delta-detection for edits; any secret for new writes). Findings the +// user already suppressed via `cx ignore-vulnerability` are filtered by the scanner. Fail-open on +// infrastructure errors. Fail closed when the post-edit content cannot be reconstructed. +// The user-visible reason never contains the secret value. +func ScanFileEdit(ev *agenthooks.FileEditEvent, svc *Scanner, telemetryWrapper wrappers.TelemetryWrapper, agent string) (blocked bool, reason, context, severity string) { + if svc == nil || svc.scan == nil || ev == nil { + return false, "", "", "" + } + + findingCount := 0 + defer func() { + if r := recover(); r != nil { + logger.PrintfIfVerbose("secrets guardrail: recovered from panic, failing open: %v", r) + blocked = false + reason = "" + context = "" + severity = "" + } + logSecretsTelemetry(telemetryWrapper, agent, ev.SessionID, findingCount) + }() + + newContent, originalContent, ok := proposedContent(ev.FilePath, ev.Changes) + if !ok { + findingCount = 1 + return true, provenanceTag() + " " + unresolvedEditReason, provenanceTag() + " " + unresolvedEditContext, "" + } + if newContent == "" { + return false, "", "", "" + } + + ignoreFilePath := existingIgnoreFilePath(ev.WorkDir) + newResults, err := svc.scan(ev.FilePath, newContent, ignoreFilePath) + if err != nil { + logger.PrintfIfVerbose("secrets guardrail: scan of proposed content failed, failing open: %v", err) + return false, "", "", "" + } + if len(newResults) == 0 { + return false, "", "", "" + } + + findings := newResults + if originalContent != "" { + origResults, origErr := svc.scan(ev.FilePath, originalContent, ignoreFilePath) + if origErr != nil { + logger.PrintfIfVerbose("secrets guardrail: scan of original content failed, failing open: %v", origErr) + return false, "", "", "" + } + findings = NewFindings(origResults, newResults) + if len(findings) == 0 { + return false, "", "", "" + } + } + + findings = aboveThreshold(findings) + if len(findings) == 0 { + return false, "", "", "" + } + + r, c := formatFindings(ev.FilePath, findings, ev.Agent, ev.WorkDir, ev.SessionID) + findingCount = len(findings) + return true, r, c, highestSeverity(findings) +} + +// aboveThreshold drops findings under the developer's severity threshold. +func aboveThreshold(findings []secretsrealtime.SecretsRealtimeResult) []secretsrealtime.SecretsRealtimeResult { + if BelowThreshold == nil { + return findings + } + kept := make([]secretsrealtime.SecretsRealtimeResult, 0, len(findings)) + for i := range findings { + if !BelowThreshold(findings[i].Severity) { + kept = append(kept, findings[i]) + } + } + return kept +} + +func highestSeverity(findings []secretsrealtime.SecretsRealtimeResult) string { + rank := map[string]int{"Critical": 4, "High": 3, "Medium": 2, "Low": 1} + best := "" + bestRank := -1 + for i := range findings { + if r, ok := rank[findings[i].Severity]; ok && r > bestRank { + bestRank = r + best = findings[i].Severity + } + } + return best +} + +func logSecretsTelemetry(telemetryWrapper wrappers.TelemetryWrapper, agent, sessionID string, totalCount int) { + if telemetryWrapper == nil || totalCount == 0 { + return + } + + telemetryData := &wrappers.DataForAITelemetry{ + Agent: agent + "-cli", + AIProvider: agent, + Engine: "Secrets", + TotalCount: totalCount, + UniqueID: wrappers.GetUniqueID(), + Type: "hooks-detect", + SubType: "scan", + ScanType: "secrets", + AiAgentSessionId: sessionID, + } + + if err := telemetryWrapper.SendAIDataToLog(telemetryData); err != nil { + logger.PrintfIfVerbose("secrets guardrail: failed to send telemetry: %v", err) + } +} + +// existingIgnoreFilePath returns the realtime ignore-file path anchored at workDir only +// when it exists on disk. A missing file means the user has not suppressed anything yet. +func existingIgnoreFilePath(workDir string) string { + p := ignore.PathFor(workDir) + if _, err := os.Stat(p); err == nil { + return p + } + return "" +} diff --git a/internal/commands/agenthooks/guardrails/secrets/secrets_test.go b/internal/commands/agenthooks/guardrails/secrets/secrets_test.go new file mode 100644 index 000000000..c39e6f0d2 --- /dev/null +++ b/internal/commands/agenthooks/guardrails/secrets/secrets_test.go @@ -0,0 +1,330 @@ +//go:build !integration + +package secrets + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + agenthooks "github.com/Checkmarx/ast-cx-hooks" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine" + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/secretsrealtime" + "github.com/checkmarx/ast-cli/internal/wrappers" +) + +const sampleSecret = "ghp_testvalue_should_stay_out_of_verdict" + +func sampleFinding() secretsrealtime.SecretsRealtimeResult { + return secretsrealtime.SecretsRealtimeResult{ + Title: "github-pat", + Description: "GitHub personal access token", + SecretValue: sampleSecret, + Severity: "Critical", + FilePath: "app.env", + Locations: []realtimeengine.Location{{Line: 4}}, + } +} + +type recordingTelemetry struct { + calls []*wrappers.DataForAITelemetry +} + +func (r *recordingTelemetry) SendAIDataToLog(data *wrappers.DataForAITelemetry) error { + r.calls = append(r.calls, data) + return nil +} + +const awsKeyTitle = "aws-access-key" + +func TestNewFindings_DropsPreExisting(t *testing.T) { + old := sampleFinding() + fresh := sampleFinding() + fresh.SecretValue = "AKIA_NEW_SECRET_VALUE" + fresh.Title = awsKeyTitle + got := NewFindings([]secretsrealtime.SecretsRealtimeResult{old}, []secretsrealtime.SecretsRealtimeResult{old, fresh}) + if len(got) != 1 || got[0].Title != awsKeyTitle { + t.Fatalf("got %+v, want only the new aws-access-key finding", got) + } +} + +func TestNewFindings_NilOriginalReturnsAll(t *testing.T) { + f := sampleFinding() + got := NewFindings(nil, []secretsrealtime.SecretsRealtimeResult{f}) + if len(got) != 1 { + t.Fatalf("got %d findings, want 1", len(got)) + } +} + +func TestScanFileEdit_NewSecret_BlocksAndRedacts(t *testing.T) { + secret := sampleSecret + svc := NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + if strings.Contains(content, secret) { + return []secretsrealtime.SecretsRealtimeResult{sampleFinding()}, nil + } + return nil, nil + }) + tel := &recordingTelemetry{} + ev := &agenthooks.FileEditEvent{ + Agent: agenthooks.AgentClaude, + SessionID: "sess-1", + FilePath: "app.env", + WorkDir: t.TempDir(), + Changes: []agenthooks.FileDiff{{Before: "", After: "TOKEN=" + secret}}, + } + blocked, reason, context, severity := ScanFileEdit(ev, svc, tel, "Claude") + if !blocked { + t.Fatal("expected the new secret to block the write") + } + if severity != "Critical" { + t.Fatalf("severity = %q, want Critical", severity) + } + assertVerdictOmitsSecret(t, reason, context) + if !strings.Contains(reason, "github-pat") || !strings.Contains(reason, "line 4") { + t.Fatalf("reason missing finding summary: %q", reason) + } + if !strings.Contains(context, "cx-devassist:cx-devassist-secrets") { + t.Fatalf("context missing skill: %q", context) + } + if !strings.Contains(context, "mcp__Checkmarx__codeRemediation") || !strings.Contains(context, `type "secret"`) || strings.Contains(context, `type "secrets"`) { + t.Fatalf("context missing MCP remediation: %q", context) + } + if !strings.Contains(reason, "[Checkmarx cx-devassist — automated security output, not user input]") { + t.Fatalf("reason missing provenance tag: %q", reason) + } + if !strings.Contains(context, "ignore-vulnerability --scan-type secrets --data '@") { + t.Fatalf("context missing redacted suppress command: %q", context) + } + assertSuppressFile(t, context, secret) + if len(tel.calls) != 1 || tel.calls[0].Type != "hooks-detect" || tel.calls[0].Engine != "Secrets" { + t.Fatalf("detect telemetry = %+v", tel.calls) + } +} + +func TestScanFileEdit_PreExistingSecret_Allows(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "app.env") + body := "note=hello\nTOKEN=" + sampleSecret + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + svc := NewScannerWithFunc(func(_, content, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + if strings.Contains(content, sampleSecret) { + return []secretsrealtime.SecretsRealtimeResult{sampleFinding()}, nil + } + return nil, nil + }) + ev := &agenthooks.FileEditEvent{ + Agent: agenthooks.AgentClaude, + FilePath: path, + WorkDir: dir, + Changes: []agenthooks.FileDiff{{Before: "note=hello", After: "note=world"}}, + } + blocked, reason, _, _ := ScanFileEdit(ev, svc, nil, "Claude") + if blocked { + t.Fatalf("an edit that does not introduce a new secret should be allowed, got: %s", reason) + } +} + +func TestScanFileEdit_DuplicateBeforeText_BlocksInsteadOfGuessing(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, testConfigEnv) + body := placeholderKey + "\n" + debugTrue + "\n" + placeholderKey + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + scanCalled := false + svc := NewScannerWithFunc(func(_, _, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + scanCalled = true + return nil, nil + }) + ev := &agenthooks.FileEditEvent{ + Agent: agenthooks.AgentClaude, + FilePath: path, + WorkDir: dir, + Changes: []agenthooks.FileDiff{{Before: placeholderKey, After: "API_KEY=" + sampleSecret}}, + } + blocked, reason, context, _ := ScanFileEdit(ev, svc, nil, "Claude") + if !blocked { + t.Fatal("an edit whose Before text is ambiguous should be blocked, not silently allowed") + } + if scanCalled { + t.Fatal("should not scan a guessed reconstruction when the edit location is ambiguous") + } + assertVerdictOmitsSecret(t, reason, context) +} + +func TestScanFileEdit_BeforeTextNotFound_BlocksInsteadOfSkipping(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, testConfigEnv) + if err := os.WriteFile(path, []byte(debugTrue), 0o600); err != nil { + t.Fatal(err) + } + scanCalled := false + svc := NewScannerWithFunc(func(_, _, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + scanCalled = true + return nil, nil + }) + ev := &agenthooks.FileEditEvent{ + Agent: agenthooks.AgentClaude, + FilePath: path, + WorkDir: dir, + Changes: []agenthooks.FileDiff{{Before: "STALE_TEXT_NOT_ON_DISK", After: "TOKEN=" + sampleSecret}}, + } + blocked, reason, context, _ := ScanFileEdit(ev, svc, nil, "Claude") + if !blocked { + t.Fatal("an edit whose Before text cannot be located should be blocked, not silently dropped and allowed") + } + assertVerdictOmitsSecret(t, reason, context) + if scanCalled { + t.Fatal("should not scan content that silently omits the unresolved edit") + } +} + +func TestScanFileEdit_ScanError_FailOpen(t *testing.T) { + svc := NewScannerWithFunc(func(_, _, _ string) ([]secretsrealtime.SecretsRealtimeResult, error) { + return nil, os.ErrClosed + }) + ev := &agenthooks.FileEditEvent{ + Agent: agenthooks.AgentCodex, + FilePath: "a.txt", + Changes: []agenthooks.FileDiff{{Before: "", After: "TOKEN=" + sampleSecret}}, + } + blocked, reason, _, _ := ScanFileEdit(ev, svc, nil, "Codex") + if blocked { + t.Fatalf("scanner error should fail open, got: %s", reason) + } +} + +func TestFormatFindings_AgentSpecific(t *testing.T) { + findings := []secretsrealtime.SecretsRealtimeResult{sampleFinding()} + workDir := t.TempDir() + + _, claude := formatFindings("app.env", findings, agenthooks.AgentClaude, workDir, "s") + if !strings.Contains(claude, "cx-devassist:cx-devassist-secrets") { + t.Fatalf("claude skill missing: %q", claude) + } + if strings.Contains(claude, sampleSecret) { + t.Fatal("claude context leaked the secret") + } + + _, copilot := formatFindings("app.env", findings, agenthooks.AgentCopilotCLI, workDir, "s") + if !strings.Contains(copilot, "aiProvider=Copilot") { + t.Fatalf("copilot provenance missing: %q", copilot) + } + + _, gemini := formatFindings("app.env", findings, agenthooks.AgentGemini, workDir, "s") + if !strings.Contains(gemini, "/cx-devassist-secrets") || !strings.Contains(gemini, "mcp_Checkmarx_codeRemediation") { + t.Fatalf("gemini targets missing: %q", gemini) + } + if strings.Contains(gemini, "mcp__Checkmarx__codeRemediation") { + t.Fatal("gemini should use single-underscore MCP tool names") + } + if !strings.Contains(gemini, "run /mcp") && !strings.Contains(gemini, "restart the Gemini CLI") { + t.Fatalf("gemini reconnect hint missing: %q", gemini) + } + + _, codex := formatFindings("app.env", findings, agenthooks.AgentCodex, workDir, "s") + if !strings.Contains(codex, "restart Codex") { + t.Fatalf("codex reconnect hint missing: %q", codex) + } + + _, cursor := formatFindings("app.env", findings, agenthooks.AgentCursor, workDir, "s") + if !strings.Contains(cursor, "cx-devassist-secrets.mdc") || !strings.Contains(cursor, "This is a Checkmarx hook deny") { + t.Fatalf("cursor prompt missing: %q", cursor) + } + if strings.Contains(cursor, "ASK THE USER FIRST") { + t.Fatal("cursor prompt still asks the user first") + } + if !strings.Contains(cursor, "mcp__plugin-cx-devassist-Checkmarx__codeRemediation") { + t.Fatalf("cursor MCP tool missing: %q", cursor) + } + if strings.Contains(cursor, sampleSecret) { + t.Fatal("cursor context leaked the secret") + } +} + +func TestFormatFindings_OmitsInjectionTriggers(t *testing.T) { + findings := []secretsrealtime.SecretsRealtimeResult{sampleFinding()} + workDir := t.TempDir() + agents := []agenthooks.AgentID{ + agenthooks.AgentClaude, + agenthooks.AgentCodex, + agenthooks.AgentCopilot, + agenthooks.AgentCursor, + agenthooks.AgentGemini, + } + for _, agent := range agents { + _, ctx := formatFindings("app.env", findings, agent, workDir, "s1") + for _, bad := range []string{"without asking", "silently", "cx_mcp_register", "the hook on that retry is the check"} { + if strings.Contains(ctx, bad) { + t.Errorf("%s context contains %q", agent, bad) + } + } + if !strings.Contains(ctx, "This is a Checkmarx hook deny") { + t.Errorf("%s context missing hook deny header", agent) + } + if !strings.Contains(ctx, "that still needs (a) or (b)") { + t.Errorf("%s context missing suppression label", agent) + } + if !strings.Contains(ctx, "cx scan secrets-realtime -s") { + t.Errorf("%s context missing secrets re-scan", agent) + } + if !strings.Contains(ctx, "that re-scan validates the finding") { + t.Errorf("%s context missing re-scan validation", agent) + } + if !strings.Contains(ctx, "partial fix") { + t.Errorf("%s context missing partial-fix status", agent) + } + if !strings.Contains(ctx, "Stop after 3 denied retries") { + t.Errorf("%s context missing retry stop", agent) + } + if !strings.Contains(ctx, "Checkmarx DevAssist Secret Remediation Summary") { + t.Errorf("%s context missing remediation summary", agent) + } + } +} + +func TestSafeDescription_RedactsEmbeddedSecret(t *testing.T) { + f := sampleFinding() + f.Description = "token " + sampleSecret + " found" + if safeDescription(&f) != genericSecretDescription { + t.Fatal("description that embeds the secret should be redacted") + } +} + +func assertVerdictOmitsSecret(t *testing.T, reason, context string) { + t.Helper() + if strings.Contains(reason, sampleSecret) || strings.Contains(context, sampleSecret) { + t.Fatal("verdict leaked the secret value") + } +} + +func assertSuppressFile(t *testing.T, context, secret string) { + t.Helper() + const marker = "--data '@" + idx := strings.Index(context, marker) + if idx < 0 { + t.Fatalf("suppress command not found in %q", context) + } + rest := context[idx+len(marker):] + end := strings.Index(rest, "'") + if end < 0 { + t.Fatal("unterminated suppress path") + } + path := strings.TrimPrefix(rest[:end], "@") + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var got secretsrealtime.IgnoredSecret + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got.Title != "github-pat" || got.SecretValue != secret { + t.Fatalf("suppress file = %+v", got) + } + t.Cleanup(func() { _ = os.Remove(path) }) +} diff --git a/internal/commands/agenthooks/sca/sca.go b/internal/commands/agenthooks/sca/sca.go index 43785fab2..641906fd4 100644 --- a/internal/commands/agenthooks/sca/sca.go +++ b/internal/commands/agenthooks/sca/sca.go @@ -6,6 +6,11 @@ import ( "github.com/checkmarx/ast-cli/internal/services/realtimeengine/ossrealtime" ) +// BelowThreshold is set by the hook dispatcher and reports whether a finding's severity is under the +// developer's cx_severity_threshold. Such findings are dropped from the deny, so the agent is never +// told to remediate them. Nil (the default) keeps every finding. +var BelowThreshold func(severity string) bool + // CheckBashInstall is the entry point for the pre-Bash-tool guardrail. It // returns ("", "", "") to allow the command, or (finding, remediation, severity) to block. // Errors fail open — we never block on infrastructure failures. @@ -73,6 +78,7 @@ func denyFrom(malicious, vulnerable []ossrealtime.OssPackage, workDir, agent, se f, r := DenyMalicious(malicious, agent) return tag + " " + f, tag + " " + r, statusMalicious } + vulnerable = aboveThreshold(vulnerable) if len(vulnerable) > 0 { f, r := DenyVulnerable(vulnerable, workDir, agent, sessionID) return tag + " " + f, tag + " " + r, highestVulnSeverity(vulnerable) @@ -80,6 +86,29 @@ func denyFrom(malicious, vulnerable []ossrealtime.OssPackage, workDir, agent, se return "", "", "" } +// aboveThreshold drops CVEs below the developer's severity threshold, and any package left with +// none. Malicious packages never go through here. A CVE with an unknown severity is kept. +func aboveThreshold(pkgs []ossrealtime.OssPackage) []ossrealtime.OssPackage { + if BelowThreshold == nil { + return pkgs + } + kept := make([]ossrealtime.OssPackage, 0, len(pkgs)) + for i := range pkgs { + p := pkgs[i] + vulns := make([]ossrealtime.Vulnerability, 0, len(p.Vulnerabilities)) + for _, v := range p.Vulnerabilities { + if !BelowThreshold(v.Severity) { + vulns = append(vulns, v) + } + } + if len(vulns) > 0 { + p.Vulnerabilities = vulns + kept = append(kept, p) + } + } + return kept +} + // highestVulnSeverity returns the highest CVE severity across all vulnerable packages. func highestVulnSeverity(pkgs []ossrealtime.OssPackage) string { rank := map[string]int{"Critical": 4, "High": 3, "Medium": 2, "Low": 1} diff --git a/internal/commands/agenthooks/sca/threshold_test.go b/internal/commands/agenthooks/sca/threshold_test.go new file mode 100644 index 000000000..320a45521 --- /dev/null +++ b/internal/commands/agenthooks/sca/threshold_test.go @@ -0,0 +1,42 @@ +//go:build !integration + +package sca + +import ( + "testing" + + "github.com/checkmarx/ast-cli/internal/services/realtimeengine/ossrealtime" +) + +func TestAboveThreshold_DropsLowCVEsAndEmptyPackages(t *testing.T) { + old := BelowThreshold + t.Cleanup(func() { BelowThreshold = old }) + BelowThreshold = func(sev string) bool { return sev == "Low" || sev == "Medium" } + + pkgs := []ossrealtime.OssPackage{ + {PackageName: "mixed", Vulnerabilities: []ossrealtime.Vulnerability{{CVE: "CVE-1", Severity: "High"}, {CVE: "CVE-2", Severity: "Medium"}}}, + {PackageName: "lowonly", Vulnerabilities: []ossrealtime.Vulnerability{{CVE: "CVE-3", Severity: "Low"}}}, + {PackageName: "unknown", Vulnerabilities: []ossrealtime.Vulnerability{{CVE: "CVE-4", Severity: ""}}}, + } + got := aboveThreshold(pkgs) + if len(got) != 2 || got[0].PackageName != "mixed" || got[1].PackageName != "unknown" { + t.Fatalf("unexpected packages: %+v", got) + } + if len(got[0].Vulnerabilities) != 1 || got[0].Vulnerabilities[0].CVE != "CVE-1" { + t.Fatalf("mixed package must keep only its High CVE: %+v", got[0].Vulnerabilities) + } + if len(pkgs[0].Vulnerabilities) != 2 { + t.Fatal("input must not be mutated") + } +} + +func TestDenyFrom_MaliciousIgnoresThreshold(t *testing.T) { + old := BelowThreshold + t.Cleanup(func() { BelowThreshold = old }) + BelowThreshold = func(string) bool { return true } + + mal := []ossrealtime.OssPackage{{PackageName: "evil", Status: statusMalicious}} + if f, _, sev := denyFrom(mal, nil, "", "Copilot", "s"); f == "" || sev != statusMalicious { + t.Fatalf("malicious must always deny, got finding=%q severity=%q", f, sev) + } +} diff --git a/internal/commands/agenthooks/scannerconfig/scannerconfig.go b/internal/commands/agenthooks/scannerconfig/scannerconfig.go new file mode 100644 index 000000000..aafaf7b2e --- /dev/null +++ b/internal/commands/agenthooks/scannerconfig/scannerconfig.go @@ -0,0 +1,121 @@ +// Package scannerconfig reads the per-scanner on/off switches a developer sets in +// ~/.checkmarx/cx-onboarding.properties (seeded by the agent plugins). Every scanner is ENABLED +// unless its key is explicitly set to a false-like value, and any read problem also means enabled: +// the switches can only narrow what the hooks do, never make them block. +package scannerconfig + +import ( + "bufio" + "io" + "os" + "path/filepath" + "strings" +) + +// Scanner identifies one guardrail engine. +type Scanner string + +const ( + // ASCA is the key for the ASCA (source code) scanner switch. + ASCA Scanner = "cx_asca_scanner_enabled" + // SCA is the key for the SCA (open source packages) scanner switch. + SCA Scanner = "cx_sca_scanner_enabled" + // KICS is the key for the KICS (IaC) scanner switch. + KICS Scanner = "cx_kics_scanner_enabled" + // Secrets is the key for the secrets scanner switch. + Secrets Scanner = "cx_secret_scanner_enabled" +) + +const ( + fileName = "cx-onboarding.properties" + // maxBytes mirrors the plugins' reader: an oversized file is treated as unreadable (all enabled). + maxBytes = 8192 + utf8BOM = "\xef\xbb\xbf" +) + +// Path returns ~/.checkmarx/cx-onboarding.properties, or "" when the home directory is unknown. +func Path() string { + home, err := os.UserHomeDir() + if err != nil || home == "" { + return "" + } + return filepath.Join(home, ".checkmarx", fileName) +} + +// Enabled reports whether the scanner is switched on in the user's config file. +func Enabled(s Scanner) bool { + return enabledIn(Path(), s) +} + +func enabledIn(path string, s Scanner) bool { + value, found := valueIn(path, string(s)) + return !found || !isFalse(value) +} + +// valueIn returns the last `key=value` for key in the file, or found=false when the file is missing, +// unreadable, oversized or does not set the key. Same parsing rules as the plugins' readers. +func valueIn(path, key string) (value string, found bool) { + if path == "" { + return "", false + } + f, err := os.Open(path) + if err != nil { + return "", false + } + defer func() { _ = f.Close() }() + raw, err := io.ReadAll(io.LimitReader(f, maxBytes+1)) + if err != nil || len(raw) > maxBytes { + return "", false + } + sc := bufio.NewScanner(strings.NewReader(strings.TrimPrefix(string(raw), utf8BOM))) + for sc.Scan() { // last occurrence wins + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + k, v, ok := strings.Cut(line, "=") + if !ok || strings.TrimSpace(k) != key { + continue + } + value, found = v, true + } + return value, found +} + +func isFalse(v string) bool { + switch strings.ToLower(strings.TrimSpace(v)) { + case "false", "0", "no", "off", "disabled": + return true + } + return false +} + +const thresholdKey = "cx_severity_threshold" + +// severityRank orders the severities the guardrails report. Anything not listed (an empty string, a +// package status such as "Malicious", an unexpected label) has no rank and is never relaxed. +var severityRank = map[string]int{"low": 1, "medium": 2, "high": 3, "critical": 4} + +// Threshold returns the minimum severity rank (1 low … 4 critical) a finding needs to block. +// Blank, missing or invalid means low, so nothing is filtered. +func Threshold() int { + return thresholdIn(Path()) +} + +func thresholdIn(path string) int { + value, found := valueIn(path, thresholdKey) + if !found { + return 1 + } + if rank, ok := severityRank[strings.ToLower(strings.TrimSpace(value))]; ok { + return rank + } + return 1 +} + +// BelowThreshold reports whether a deny whose highest severity is `severity` should be let through +// because it is under cx_severity_threshold. An unranked severity returns false (keep blocking). +func BelowThreshold(severity string) bool { + rank, ok := severityRank[strings.ToLower(strings.TrimSpace(severity))] + return ok && rank < Threshold() +} diff --git a/internal/commands/agenthooks/scannerconfig/scannerconfig_test.go b/internal/commands/agenthooks/scannerconfig/scannerconfig_test.go new file mode 100644 index 000000000..3353059f9 --- /dev/null +++ b/internal/commands/agenthooks/scannerconfig/scannerconfig_test.go @@ -0,0 +1,138 @@ +//go:build !integration + +package scannerconfig + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func write(t *testing.T, body string) string { + t.Helper() + p := filepath.Join(t.TempDir(), fileName) + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + return p +} + +func TestEnabledDefaults(t *testing.T) { + if !enabledIn("", ASCA) { + t.Error("empty path must mean enabled") + } + if !enabledIn(filepath.Join(t.TempDir(), "missing"), SCA) { + t.Error("missing file must mean enabled") + } + if !enabledIn(write(t, "# nothing set\n"), KICS) { + t.Error("absent key must mean enabled") + } +} + +func TestEnabledFalseValues(t *testing.T) { + for _, v := range []string{"false", "FALSE", "0", "no", "off", "Disabled", " false "} { + if enabledIn(write(t, "cx_asca_scanner_enabled="+v+"\n"), ASCA) { + t.Errorf("%q should disable", v) + } + } +} + +func TestEnabledGarbageStaysOn(t *testing.T) { + for _, v := range []string{"flase", "", "banana", "2", "true"} { + if !enabledIn(write(t, "cx_kics_scanner_enabled="+v+"\n"), KICS) { + t.Errorf("%q must stay enabled", v) + } + } +} + +func TestKeysAreIndependent(t *testing.T) { + p := write(t, "cx_sca_scanner_enabled=false\ncx_secret_scanner_enabled=true\n") + if enabledIn(p, SCA) || !enabledIn(p, ASCA) || !enabledIn(p, KICS) || !enabledIn(p, Secrets) { + t.Error("only SCA should be disabled") + } +} + +func TestCommentedLineIgnoredAndLastWins(t *testing.T) { + if !enabledIn(write(t, "# cx_asca_scanner_enabled=false\n"), ASCA) { + t.Error("commented key must be ignored") + } + if !enabledIn(write(t, "cx_asca_scanner_enabled=false\ncx_asca_scanner_enabled=true\n"), ASCA) { + t.Error("last occurrence should win") + } +} + +func TestBOMAndCRLF(t *testing.T) { + p := write(t, "\xef\xbb\xbfcx_secret_scanner_enabled=false\r\ncx_asca_scanner_enabled=true\r\n") + if enabledIn(p, Secrets) || !enabledIn(p, ASCA) { + t.Error("BOM/CRLF file misparsed") + } +} + +func TestOversizedFailsOpen(t *testing.T) { + p := write(t, strings.Repeat("# pad\n", 5000)+"cx_asca_scanner_enabled=false\n") + if !enabledIn(p, ASCA) { + t.Error("oversized file must fail open to enabled") + } +} + +func TestPathUsesHome(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + if got := Path(); got != filepath.Join(dir, ".checkmarx", fileName) { + t.Errorf("unexpected path %q", got) + } + _ = os.MkdirAll(filepath.Join(dir, ".checkmarx"), 0o755) + _ = os.WriteFile(Path(), []byte("cx_kics_scanner_enabled=off\n"), 0o600) + if Enabled(KICS) || !Enabled(ASCA) { + t.Error("Enabled should read the file under the user's home") + } +} + +func TestThresholdIn(t *testing.T) { + cases := map[string]int{ + "": 1, // no file content + "cx_severity_threshold=\n": 1, // blank + "cx_severity_threshold=low\n": 1, + "cx_severity_threshold=Medium\n": 2, + "cx_severity_threshold= HIGH \n": 3, + "cx_severity_threshold=critical\n": 4, + "cx_severity_threshold=hihg\n": 1, // invalid + "# cx_severity_threshold=high\n": 1, // commented + "cx_severity_threshold=high\ncx_severity_threshold=low\n": 1, // last wins + } + for body, want := range cases { + if got := thresholdIn(write(t, body)); got != want { + t.Errorf("%q: got %d, want %d", body, got, want) + } + } + if thresholdIn(filepath.Join(t.TempDir(), "missing")) != 1 || thresholdIn("") != 1 { + t.Error("missing file / path must mean low") + } +} + +func TestBelowThreshold(t *testing.T) { + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) + _ = os.MkdirAll(filepath.Join(dir, ".checkmarx"), 0o755) + set := func(body string) { + if err := os.WriteFile(Path(), []byte(body), 0o600); err != nil { + t.Fatal(err) + } + } + set("cx_severity_threshold=high\n") + for sev, want := range map[string]bool{ + "Low": true, "MEDIUM": true, "High": false, "Critical": false, + "": false, "Malicious": false, "weird": false, // unranked is never relaxed + } { + if got := BelowThreshold(sev); got != want { + t.Errorf("high threshold, %q: got %v, want %v", sev, got, want) + } + } + set("cx_severity_threshold=\n") + if BelowThreshold("Low") { + t.Error("blank threshold must filter nothing") + } +} diff --git a/internal/services/realtimeengine/secretsrealtime/secrets-realtime.go b/internal/services/realtimeengine/secretsrealtime/secrets-realtime.go index a4ab60914..406261caf 100644 --- a/internal/services/realtimeengine/secretsrealtime/secrets-realtime.go +++ b/internal/services/realtimeengine/secretsrealtime/secrets-realtime.go @@ -80,13 +80,8 @@ func (s *SecretsRealtimeService) RunSecretsRealtimeScan(filePath, ignoredFilePat return nil, errorconstants.NewRealtimeEngineError(errorconstants.RealtimeEngineFilePathRequired).Error() } - if enabled, err := realtimeengine.IsFeatureFlagEnabled(s.FeatureFlagWrapper, wrappers.OssRealtimeEnabled); err != nil || !enabled { - logger.PrintfIfVerbose("Failed to print OSS Realtime scan results: %v", err) - return nil, errorconstants.NewRealtimeEngineError(errorconstants.RealtimeEngineNotAvailable).Error() - } - - if err := realtimeengine.EnsureLicense(s.JwtWrapper); err != nil { - return nil, errorconstants.NewRealtimeEngineError(err.Error()).Error() + if err := s.ensureReady(); err != nil { + return nil, err } if err := realtimeengine.ValidateFilePath(filePath); err != nil { @@ -100,7 +95,35 @@ func (s *SecretsRealtimeService) RunSecretsRealtimeScan(filePath, ignoredFilePat return nil, errorconstants.NewRealtimeEngineError("failed to read file").Error() } - report, err := runScan(filePath, content) + return finishScan(filePath, content, ignoredFilePath) +} + +// ScanContent runs the secrets scanner on content that is not yet on disk. +// sourcePath is reported as the finding file path (the agent's target file). +// ignoredFilePath filters findings the user suppressed via cx ignore-vulnerability. +func (s *SecretsRealtimeService) ScanContent(sourcePath, content, ignoredFilePath string) ([]SecretsRealtimeResult, error) { + if sourcePath == "" { + return nil, errorconstants.NewRealtimeEngineError(errorconstants.RealtimeEngineFilePathRequired).Error() + } + if err := s.ensureReady(); err != nil { + return nil, err + } + return finishScan(sourcePath, content, ignoredFilePath) +} + +func (s *SecretsRealtimeService) ensureReady() error { + if enabled, err := realtimeengine.IsFeatureFlagEnabled(s.FeatureFlagWrapper, wrappers.OssRealtimeEnabled); err != nil || !enabled { + logger.PrintfIfVerbose("Failed to print OSS Realtime scan results: %v", err) + return errorconstants.NewRealtimeEngineError(errorconstants.RealtimeEngineNotAvailable).Error() + } + if err := realtimeengine.EnsureLicense(s.JwtWrapper); err != nil { + return errorconstants.NewRealtimeEngineError(err.Error()).Error() + } + return nil +} + +func finishScan(sourcePath, content, ignoredFilePath string) ([]SecretsRealtimeResult, error) { + report, err := runScan(sourcePath, content) if err != nil { logger.PrintfIfVerbose("Failed to run scan: %v", err) return nil, errorconstants.NewRealtimeEngineError("failed to run secrets scan").Error() @@ -109,6 +132,9 @@ func (s *SecretsRealtimeService) RunSecretsRealtimeScan(filePath, ignoredFilePat results := convertToSecretsRealtimeResult(report) resultsPerLineMap := createResultsPerLocationMap(results) results = filterGenericAPIKeyVulIfNeeded(results, resultsPerLineMap) + for i := range results { + results[i].FilePath = sourcePath + } if ignoredFilePath == "" { return results, nil @@ -118,9 +144,7 @@ func (s *SecretsRealtimeService) RunSecretsRealtimeScan(filePath, ignoredFilePat logger.PrintfIfVerbose("secrets-realtime: failed to load ignore file %s: %v; continuing without ignore filtering", ignoredFilePath, err) return results, nil } - ignoreMap := buildIgnoreMap(ignoredSecrets) - results = filterIgnoredSecrets(results, ignoreMap) - return results, nil + return filterIgnoredSecrets(results, buildIgnoreMap(ignoredSecrets)), nil } func readFile(filePath string) (string, error) { diff --git a/internal/services/realtimeengine/secretsrealtime/secrets-realtime_test.go b/internal/services/realtimeengine/secretsrealtime/secrets-realtime_test.go index 37a78ca1b..4eb3b93fc 100644 --- a/internal/services/realtimeengine/secretsrealtime/secrets-realtime_test.go +++ b/internal/services/realtimeengine/secretsrealtime/secrets-realtime_test.go @@ -344,3 +344,27 @@ func TestGetSeverity_EmptyStatus_ReturnsHigh(t *testing.T) { assert.Equal(t, highSeverity, severity) } + +func TestScanContent_ReportsSourcePathAndHonorsIgnoreFile(t *testing.T) { + mock.Flag = wrappers.FeatureFlagResponseModel{Name: wrappers.OssRealtimeEnabled, Status: true} + + service := &SecretsRealtimeService{ + JwtWrapper: &mock.JWTMockWrapper{}, + FeatureFlagWrapper: &mock.FeatureFlagsMockWrapper{}, + } + content := "token = \"ghp_1234567890abcdef1234567890abcdef12345678\"" + results, err := service.ScanContent("app.env", content, "") + assert.NoError(t, err) + assert.NotEmpty(t, results) + assert.Equal(t, "app.env", results[0].FilePath) + assert.Equal(t, "github-pat", results[0].Title) + + raw, err := json.Marshal([]IgnoredSecret{{Title: results[0].Title, SecretValue: results[0].SecretValue}}) + assert.NoError(t, err) + ignoreFile := filepath.Join(t.TempDir(), "ignore.json") + assert.NoError(t, os.WriteFile(ignoreFile, raw, 0600)) + + filtered, err := service.ScanContent("app.env", content, ignoreFile) + assert.NoError(t, err) + assert.Empty(t, filtered) +}