diff --git a/internal/configure/configure.go b/internal/configure/configure.go index ea7fb47..80a6494 100644 --- a/internal/configure/configure.go +++ b/internal/configure/configure.go @@ -23,6 +23,16 @@ var snykSkillsAlwaysApply string //go:embed skills/sast/smart_apply.md var snykSkillsSmartApply string +// Claude Code's user-level rules loader (~/.claude/rules/*.md) does not use +// the Cursor SKILL.md frontmatter (name/description); these are clean +// markdown bodies tailored for that loader. +// +//go:embed rules/claude/always_apply.md +var snykClaudeRulesAlwaysApply string + +//go:embed rules/claude/smart_apply.md +var snykClaudeRulesSmartApply string + // Configure sets up MCP server and rules for the specified IDE host. func Configure(logger *zerolog.Logger, config configuration.Configuration, userInterface ui.UserInterface, cliPath string) error { hostName := config.GetString(shared.ToolNameParam) @@ -80,6 +90,19 @@ func removeConfiguration(logger *zerolog.Logger, config configuration.Configurat logger.Info().Msgf("Successfully removed global skills for %s from %s", ideConf.name, ideConf.globalSkillsPath) } + // Remove dedicated rules file (e.g. claude-cli) + if ideConf.globalDedicatedRulesPath != "" { + _ = userInterface.Output(fmt.Sprintf("šŸ“‹ Removing dedicated rules file from: %s", ideConf.globalDedicatedRulesPath)) + + err := removeGlobalSkills(ideConf.globalDedicatedRulesPath, logger) + if err != nil { + return fmt.Errorf("failed to remove dedicated rules file for %s: %w", ideConf.name, err) + } + + _ = userInterface.Output(fmt.Sprintf("āœ… Successfully removed dedicated rules file for %s", ideConf.name)) + logger.Info().Msgf("Successfully removed dedicated rules file for %s from %s", ideConf.name, ideConf.globalDedicatedRulesPath) + } + // Remove global rules (e.g. Windsurf, Antigravity, gemini-cli, claude-cli) if rulesScope == shared.RulesGlobalScope && ideConf.globalRulesPath != "" { _ = userInterface.Output(fmt.Sprintf("šŸ“‹ Removing global rules from: %s", ideConf.globalRulesPath)) @@ -106,15 +129,16 @@ func removeConfiguration(logger *zerolog.Logger, config configuration.Configurat logger.Info().Msgf("Successfully removed local rules for %s", ideConf.name) } - // Clean up legacy local rules - if ideConf.legacyLocalRulesPath != "" && workspacePath != "" { - err := removeLocalRules(workspacePath, ideConf.legacyLocalRulesPath, logger) - if err != nil { - logger.Warn().Err(err).Msgf("Unable to clean up legacy local rules at %s", ideConf.legacyLocalRulesPath) - } - } + cleanupLegacyLocalRules(logger, workspacePath, ideConf) } + // Migration housekeeping runs OUTSIDE the configureRules gate: a user + // invoking remove with configureRules=false (e.g. wanting to remove only + // the MCP entry) still expects stale Snyk content in their personal + // CLAUDE.md to be cleaned up ā€” migration is independent of whether rules + // are configured this run. + cleanupLegacyGlobalRules(logger, userInterface, ideConf) + _ = userInterface.Output("\nšŸŽ‰ Removal complete!") _ = userInterface.Output("\nNext steps:") _ = userInterface.Output(fmt.Sprintf(" 1. Restart %s to apply the changes", ideConf.name)) @@ -207,6 +231,27 @@ func addConfiguration(logger *zerolog.Logger, config configuration.Configuration logger.Info().Msgf("Successfully wrote global skills for %s at %s", ideConf.name, ideConf.globalSkillsPath) } + // Write dedicated rules file (e.g. claude-cli ~/.claude/rules/snyk-security.md) + if ideConf.globalDedicatedRulesPath != "" { + var dedicatedRulesContent string + switch ruleType { + case shared.RuleTypeAlwaysApply: + dedicatedRulesContent = snykClaudeRulesAlwaysApply + case shared.RuleTypeSmart: + dedicatedRulesContent = snykClaudeRulesSmartApply + } + + _ = userInterface.Output(fmt.Sprintf("šŸ“‹ Writing dedicated rules file (%s) to: %s", ruleType, ideConf.globalDedicatedRulesPath)) + + err := writeGlobalSkills(ideConf.globalDedicatedRulesPath, dedicatedRulesContent, logger) + if err != nil { + return fmt.Errorf("failed to write dedicated rules file for %s: %w", ideConf.name, err) + } + + _ = userInterface.Output(fmt.Sprintf("āœ… Successfully wrote dedicated rules file for %s", ideConf.name)) + logger.Info().Msgf("Successfully wrote dedicated rules file for %s at %s", ideConf.name, ideConf.globalDedicatedRulesPath) + } + // Write global rules with delimiters (e.g. Windsurf, Antigravity, gemini-cli, claude-cli) if rulesScope == shared.RulesGlobalScope && ideConf.globalRulesPath != "" { _ = userInterface.Output(fmt.Sprintf("šŸ“‹ Writing global rules (%s) to: %s", ruleType, ideConf.globalRulesPath)) @@ -238,15 +283,16 @@ func addConfiguration(logger *zerolog.Logger, config configuration.Configuration logger.Info().Msgf("Successfully wrote local rules for %s", ideConf.name) } - // Clean up legacy local rules when migrating to global rules/skills - if ideConf.legacyLocalRulesPath != "" && workspacePath != "" { - err := removeLocalRules(workspacePath, ideConf.legacyLocalRulesPath, logger) - if err != nil { - logger.Warn().Err(err).Msgf("Unable to clean up legacy local rules at %s", ideConf.legacyLocalRulesPath) - } - } + cleanupLegacyLocalRules(logger, workspacePath, ideConf) } + // Migration housekeeping runs OUTSIDE the configureRules gate (see + // cleanupLegacyGlobalRules): a user running configure with + // configureRules=false (MCP-only) has still abandoned the legacy + // location, so the stale block in their personal CLAUDE.md should be + // cleaned up regardless of whether rules are being configured this run. + cleanupLegacyGlobalRules(logger, userInterface, ideConf) + _ = userInterface.Output("\nšŸŽ‰ Configuration complete!") _ = userInterface.Output("\nNext steps:") _ = userInterface.Output(fmt.Sprintf(" 1. Restart %s to apply the changes", ideConf.name)) @@ -255,6 +301,42 @@ func addConfiguration(logger *zerolog.Logger, config configuration.Configuration return nil } +// cleanupLegacyLocalRules removes a workspace-relative legacy rules file +// left behind by an older install (e.g. .cursor/rules/snyk_rules.mdc from +// before the move to global skills). Best-effort: errors are logged at warn +// level and execution continues. Quiet by design ā€” local-rules cleanup +// piggybacks on the rules-write call site and doesn't warrant its own user +// breadcrumb. +func cleanupLegacyLocalRules(logger *zerolog.Logger, workspacePath string, ideConf *hostConfig) { + if ideConf.legacyLocalRulesPath == "" || workspacePath == "" { + return + } + err := removeLocalRules(workspacePath, ideConf.legacyLocalRulesPath, logger) + if err != nil { + logger.Warn().Err(err).Msgf("Unable to clean up legacy local rules at %s", ideConf.legacyLocalRulesPath) + } +} + +// cleanupLegacyGlobalRules strips a Snyk delimited block left behind in a +// legacy shared file (e.g. ~/.claude/CLAUDE.md from claude-cli installs +// before the move to ~/.claude/rules/). Mutating a user-owned file is the +// kind of thing the user should see in the CLI output, so this helper +// announces the cleanup before running and surfaces any failure with a +// concrete pointer back to the legacy file. removeGlobalRules is +// idempotent (no-op when the delimited block is absent), so calling this +// on every configure/remove run is safe. +func cleanupLegacyGlobalRules(logger *zerolog.Logger, userInterface ui.UserInterface, ideConf *hostConfig) { + if ideConf.legacyGlobalRulesPath == "" { + return + } + _ = userInterface.Output(fmt.Sprintf("šŸ“‹ Cleaning up legacy global rules block in: %s", ideConf.legacyGlobalRulesPath)) + err := removeGlobalRules(ideConf.legacyGlobalRulesPath, logger) + if err != nil { + logger.Warn().Err(err).Msgf("Unable to clean up legacy global rules at %s", ideConf.legacyGlobalRulesPath) + _ = userInterface.Output(fmt.Sprintf("āš ļø Failed to clean up legacy global rules at %s ā€” please inspect this file manually for the Snyk delimited block.", ideConf.legacyGlobalRulesPath)) + } +} + // determineCommand returns the command and args based on execution context func determineCommand(cliPath, integrationName string) (string, []string) { if utils.IsRunningFromNpm(integrationName) { diff --git a/internal/configure/configure_test.go b/internal/configure/configure_test.go index 32a8292..30c6fa3 100644 --- a/internal/configure/configure_test.go +++ b/internal/configure/configure_test.go @@ -5,16 +5,30 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "testing" "github.com/go-git/go-git/v5" "github.com/rs/zerolog" + "github.com/snyk/go-application-framework/pkg/configuration" + "github.com/snyk/go-application-framework/pkg/ui" "github.com/snyk/studio-mcp/shared" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// noopUserInterface satisfies ui.UserInterface for tests; every method +// discards. configure.go only calls Output(), but the interface contract +// requires the rest, so each method returns a benign zero value. +type noopUserInterface struct{} + +func (noopUserInterface) Output(string) error { return nil } +func (noopUserInterface) OutputError(error, ...ui.Opts) error { return nil } +func (noopUserInterface) NewProgressBar() ui.ProgressBar { return nil } +func (noopUserInterface) Input(string) (string, error) { return "", nil } +func (noopUserInterface) SelectOptions(string, []string) (int, string, error) { return 0, "", nil } + func TestUpsertDelimitedBlock(t *testing.T) { tests := []struct { name string @@ -52,15 +66,17 @@ func TestGetHostConfig(t *testing.T) { require.NoError(t, err) tests := []struct { - name string - hostName string - expectError bool - expectedName string - expectMcpGlobalConfig bool - expectLocalRulesPath bool - expectGlobalRulesPath bool - expectGlobalSkillsPath bool - expectLegacyLocalRulesPath bool + name string + hostName string + expectError bool + expectedName string + expectMcpGlobalConfig bool + expectLocalRulesPath bool + expectGlobalRulesPath bool + expectGlobalSkillsPath bool + expectGlobalDedicatedRulesPath bool + expectLegacyLocalRulesPath bool + expectLegacyGlobalRulesPath bool }{ { name: "cursor", @@ -113,12 +129,13 @@ func TestGetHostConfig(t *testing.T) { expectGlobalRulesPath: true, }, { - name: "claude-cli", - hostName: "claude-cli", - expectError: false, - expectedName: "claude-cli", - expectMcpGlobalConfig: true, - expectGlobalRulesPath: true, + name: "claude-cli", + hostName: "claude-cli", + expectError: false, + expectedName: "claude-cli", + expectMcpGlobalConfig: true, + expectGlobalDedicatedRulesPath: true, + expectLegacyGlobalRulesPath: true, }, { name: "unsupported", @@ -152,13 +169,81 @@ func TestGetHostConfig(t *testing.T) { assert.NotEmpty(t, config.globalSkillsPath) assert.Contains(t, config.globalSkillsPath, homeDir) } + if tt.expectGlobalDedicatedRulesPath { + assert.NotEmpty(t, config.globalDedicatedRulesPath) + assert.Contains(t, config.globalDedicatedRulesPath, homeDir) + } if tt.expectLegacyLocalRulesPath { assert.NotEmpty(t, config.legacyLocalRulesPath) } + if tt.expectLegacyGlobalRulesPath { + assert.NotEmpty(t, config.legacyGlobalRulesPath) + assert.Contains(t, config.legacyGlobalRulesPath, homeDir) + } }) } } +func TestGetHostConfig_ClaudeCliPaths(t *testing.T) { + homeDir, err := os.UserHomeDir() + require.NoError(t, err) + + config, err := getHostConfig("claude-cli") + require.NoError(t, err) + + // MCP server entry continues to live in ~/.claude.json + assert.Equal(t, filepath.Join(homeDir, ".claude.json"), config.mcpGlobalConfigPath) + + // Rules live in a dedicated, auto-loaded file under ~/.claude/rules/ + // (per https://code.claude.com/docs/en/memory#user-level-rules) ā€” distinct + // from globalSkillsPath, which is reserved for Cursor's SKILL.md slot. + assert.Equal(t, filepath.Join(homeDir, ".claude", "rules", "snyk-security.md"), config.globalDedicatedRulesPath) + + // CLAUDE.md is preserved as a legacy path so prior installs get cleaned up. + assert.Equal(t, filepath.Join(homeDir, ".claude", "CLAUDE.md"), config.legacyGlobalRulesPath) + + // Delimited globalRulesPath is no longer used for claude-cli ā€” the rules + // file is now Snyk-owned and should be written without delimiters. + assert.Empty(t, config.globalRulesPath) + + // globalSkillsPath is the Cursor-specific SKILL.md slot; claude-cli must + // not piggyback on it (we use the dedicated rules path instead). + assert.Empty(t, config.globalSkillsPath) + + // claude-cli has never used local workspace rules. + assert.Empty(t, config.localRulesPath) + assert.Empty(t, config.legacyLocalRulesPath) +} + +func TestRemoveGlobalRulesIsClaudeCliMigrationSafe(t *testing.T) { + // Verifies the migration path: a CLAUDE.md containing both pre-existing user + // content AND a Snyk delimited block (left by an older install) gets the + // Snyk block stripped while user content is preserved verbatim. This is the + // behavior addConfiguration / removeConfiguration rely on for claude-cli. + nopLogger := zerolog.New(io.Discard) + logger := &nopLogger + + tempDir := t.TempDir() + claudeMd := filepath.Join(tempDir, "CLAUDE.md") + + userContent := "# My personal preferences\n\n- Use tabs for indentation\n- Prefer Go over Rust\n" + snykBlock := RuleStart + "\n# Snyk Security At Inception\nDo the snyk thing.\n" + RuleEnd + combined := userContent + "\n" + snykBlock + "\n" + require.NoError(t, os.WriteFile(claudeMd, []byte(combined), 0644)) + + require.NoError(t, removeGlobalRules(claudeMd, logger)) + + result, err := os.ReadFile(claudeMd) + require.NoError(t, err) + resultStr := string(result) + + assert.Contains(t, resultStr, "Use tabs for indentation") + assert.Contains(t, resultStr, "Prefer Go over Rust") + assert.NotContains(t, resultStr, RuleStart) + assert.NotContains(t, resultStr, RuleEnd) + assert.NotContains(t, resultStr, "Snyk Security At Inception") +} + func TestGetHostConfig_VSCodePaths(t *testing.T) { configDir, err := os.UserConfigDir() require.NoError(t, err) @@ -1165,3 +1250,234 @@ func TestRemoveDelimitedBlock(t *testing.T) { }) } } + +// TestRemoveDelimitedBlock_LineAnchored_PreservesUserQuotedMarkers is a +// regression for the security finding that prior first-occurrence matching +// could destroy user content. Pre-fix: a CLAUDE.md that quoted the literal +// marker text inline (e.g. in a how-to doc) before the real Snyk block +// would have everything between the user's quote and Snyk's real end +// marker silently deleted. Post-fix (line-anchored matching): the inline +// quote does not match (markers must be on their own line), so user +// content above the real block survives intact. +func TestRemoveDelimitedBlock_LineAnchored_PreservesUserQuotedMarkers(t *testing.T) { + t.Run("user quotes marker inline in a sentence above real block", func(t *testing.T) { + userPreamble := "# How Snyk migration works\n\n" + + "Snyk wraps its rules with `" + RuleStart + "` and `" + RuleEnd + "` markers.\n" + + "Cleanup of legacy installs strips that delimited block.\n\n" + realBlock := RuleStart + "\n# Snyk Security At Inception\nReal content.\n" + RuleEnd + "\n" + userTrailer := "\n# More notes\nAfter the block.\n" + + source := userPreamble + realBlock + userTrailer + result := removeDelimitedBlock(source, RuleStart, RuleEnd) + + // User prose around the real block survives intact. + assert.Contains(t, result, "# How Snyk migration works") + assert.Contains(t, result, "Cleanup of legacy installs strips") + assert.Contains(t, result, "# More notes") + assert.Contains(t, result, "After the block.") + + // The quoted marker tokens INSIDE the user's sentence are preserved + // (they're user-authored prose, not Snyk's actual delimiters). + assert.Contains(t, result, "Snyk wraps its rules with `"+RuleStart+"`") + + // Real block content is gone. + assert.NotContains(t, result, "Real content.") + // AND no orphan Snyk block markers should remain on their own line. + // (We can't simply NotContains the marker tokens ā€” they survive in + // the user's quoted prose. Verify line-anchored absence instead.) + for _, line := range strings.Split(result, "\n") { + assert.NotEqual(t, RuleStart, line, "no orphan RuleStart line should remain") + assert.NotEqual(t, RuleEnd, line, "no orphan RuleEnd line should remain") + } + }) + + t.Run("user has marker on its own line AFTER real block ā€” last-start-with-matching-end picks the real one", func(t *testing.T) { + realBlock := RuleStart + "\n# Real Snyk\nReal content.\n" + RuleEnd + // User pasted the start marker on its own line below, e.g. in a code + // block they were writing. There's no matching end after it, so the + // real block above is the only complete pair. + userBelow := "\n\n```\n" + RuleStart + "\n```\n\nMy notes.\n" + + source := realBlock + userBelow + result := removeDelimitedBlock(source, RuleStart, RuleEnd) + + // Real block stripped. + assert.NotContains(t, result, "Real content.") + // Orphan user-pasted marker survives (it's inside a fenced code block). + assert.Contains(t, result, "```\n"+RuleStart+"\n```") + assert.Contains(t, result, "My notes.") + }) +} + +// TestConfigure_ClaudeCliEndToEnd exercises the full add ā†’ remove cycle for +// the claude-cli host. It pins three behaviors that helper-level tests do +// NOT cover: that addConfiguration writes the new dedicated rules file, +// that the new file is removed by removeConfiguration, and that the +// migration cleanup wired into configure.go actually strips the legacy +// delimited block from a pre-existing CLAUDE.md while preserving +// surrounding user content. A future refactor that drops the migration +// call sites would slip past every other test in this package ā€” this is +// the one that catches it. +func TestConfigure_ClaudeCliEndToEnd(t *testing.T) { + if runtime.GOOS == "windows" { + // os.UserHomeDir() resolves via USERPROFILE on Windows; t.Setenv("HOME",...) + // would not redirect it. The migration logic is platform-agnostic, so we + // rely on Linux/macOS for this integration test. + t.Skip("integration test relies on $HOME-driven UserHomeDir resolution") + } + + nopLogger := zerolog.New(io.Discard) + logger := &nopLogger + uiStub := noopUserInterface{} + + // Construct a fresh, isolated HOME for each subtest. addConfiguration and + // removeConfiguration both call os.UserHomeDir() via getHostConfig, which + // on Linux/macOS reads $HOME ā€” t.Setenv lets us redirect those reads + // without touching the developer's actual ~/.claude tree. + setupTempHome := func(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude"), 0755)) + return home + } + + // makeConfig returns a configuration.Configuration populated with the + // minimum keys Configure() reads. ruleType always-apply is the default + // payload Snyk ships; configureMcp+configureRules both true exercises + // the full add path. + makeConfig := func(removeMode bool) configuration.Configuration { + c := configuration.New() + c.Set(shared.ToolNameParam, "claude-cli") + c.Set(shared.RemoveParam, removeMode) + c.Set(shared.RuleTypeParam, shared.RuleTypeAlwaysApply) + c.Set(shared.RulesScopeParam, shared.RulesGlobalScope) + c.Set(shared.WorkspacePathParam, "") + c.Set(shared.ConfigureMcpParam, true) + c.Set(shared.ConfigureRulesParam, true) + return c + } + + t.Run("add: writes dedicated rules file and strips legacy CLAUDE.md block", func(t *testing.T) { + home := setupTempHome(t) + + // Pre-seed CLAUDE.md with the exact shape a prior install would have + // left behind: user content above and below a delimited Snyk block. + // Migration must remove ONLY the block. + userTop := "# My personal preferences\n\n- Always tabs, never spaces\n" + userBottom := "\n## Personal projects\n\n- Be terse\n" + legacyBlock := RuleStart + "\n# Snyk Security At Inception\nDo the snyk thing.\n" + RuleEnd + claudeMd := filepath.Join(home, ".claude", "CLAUDE.md") + require.NoError(t, os.WriteFile(claudeMd, []byte(userTop+legacyBlock+userBottom), 0644)) + + require.NoError(t, Configure(logger, makeConfig(false), uiStub, "/usr/local/bin/snyk")) + + // New dedicated rules file exists with the embedded Claude-Code rules + // content (no Cursor SKILL frontmatter). + rulesFile := filepath.Join(home, ".claude", "rules", "snyk-security.md") + assert.FileExists(t, rulesFile) + got, err := os.ReadFile(rulesFile) + require.NoError(t, err) + assert.Equal(t, snykClaudeRulesAlwaysApply, string(got), + "new rules file should contain the Claude-Code-shaped content embed") + assert.NotContains(t, string(got), "name: snyk-rules", + "Cursor SKILL frontmatter must not appear in the Claude rules file") + + // Legacy block is gone but user content survives. + md, err := os.ReadFile(claudeMd) + require.NoError(t, err) + mds := string(md) + assert.NotContains(t, mds, RuleStart, "legacy block start marker should be stripped") + assert.NotContains(t, mds, RuleEnd, "legacy block end marker should be stripped") + assert.NotContains(t, mds, "Do the snyk thing", "legacy Snyk content body should be stripped") + assert.Contains(t, mds, "Always tabs, never spaces", "user content above the legacy block must survive") + assert.Contains(t, mds, "Be terse", "user content below the legacy block must survive") + + // MCP server entry was written to ~/.claude.json. + claudeJSON := filepath.Join(home, ".claude.json") + assert.FileExists(t, claudeJSON) + jsonBytes, err := os.ReadFile(claudeJSON) + require.NoError(t, err) + var parsed McpConfig + require.NoError(t, json.Unmarshal(jsonBytes, &parsed)) + assert.Contains(t, parsed.McpServers, shared.ServerNameKey, + "~/.claude.json should contain the Snyk MCP server entry") + }) + + t.Run("remove: deletes dedicated rules file and strips legacy CLAUDE.md block", func(t *testing.T) { + home := setupTempHome(t) + + // Pre-create both the new rules file (as if a prior add had run) AND a + // CLAUDE.md still carrying the legacy block. removeConfiguration must + // take care of both. + rulesFile := filepath.Join(home, ".claude", "rules", "snyk-security.md") + require.NoError(t, os.MkdirAll(filepath.Dir(rulesFile), 0755)) + require.NoError(t, os.WriteFile(rulesFile, []byte(snykClaudeRulesAlwaysApply), 0644)) + + userContent := "# Personal\n\n- A\n- B\n" + legacyBlock := RuleStart + "\n# Old Snyk\nOld.\n" + RuleEnd + "\n" + claudeMd := filepath.Join(home, ".claude", "CLAUDE.md") + require.NoError(t, os.WriteFile(claudeMd, []byte(userContent+legacyBlock), 0644)) + + // Pre-seed an MCP server entry so removeConfiguration has something to + // strip. + claudeJSON := filepath.Join(home, ".claude.json") + seed := []byte(`{"mcpServers":{"Snyk":{"command":"/x","args":["mcp","-t","stdio"],"env":{}}}}`) + require.NoError(t, os.WriteFile(claudeJSON, seed, 0644)) + + require.NoError(t, Configure(logger, makeConfig(true), uiStub, "/usr/local/bin/snyk")) + + // Dedicated rules file is gone. + _, statErr := os.Stat(rulesFile) + assert.True(t, os.IsNotExist(statErr), "dedicated rules file should be removed") + + // Legacy block stripped, user content preserved. + md, err := os.ReadFile(claudeMd) + require.NoError(t, err) + assert.NotContains(t, string(md), RuleStart) + assert.NotContains(t, string(md), "Old Snyk") + assert.Contains(t, string(md), "# Personal") + + // MCP entry removed from ~/.claude.json. + jsonBytes, err := os.ReadFile(claudeJSON) + require.NoError(t, err) + var parsed McpConfig + require.NoError(t, json.Unmarshal(jsonBytes, &parsed)) + assert.NotContains(t, parsed.McpServers, shared.ServerNameKey) + }) + + t.Run("MCP-only configure (configureRules=false) still runs legacy cleanup", func(t *testing.T) { + // Pins F12: migration housekeeping must run regardless of whether the + // user is configuring rules this invocation. A user running + // `--configure-rules=false --configure-mcp=true` who previously + // installed the delimited variant should still get their CLAUDE.md + // cleaned up. + home := setupTempHome(t) + + userContent := "# Personal\n" + legacyBlock := RuleStart + "\n# stale snyk\n" + RuleEnd + "\n" + claudeMd := filepath.Join(home, ".claude", "CLAUDE.md") + require.NoError(t, os.WriteFile(claudeMd, []byte(userContent+legacyBlock), 0644)) + + c := configuration.New() + c.Set(shared.ToolNameParam, "claude-cli") + c.Set(shared.RemoveParam, false) + c.Set(shared.RuleTypeParam, shared.RuleTypeAlwaysApply) + c.Set(shared.RulesScopeParam, shared.RulesGlobalScope) + c.Set(shared.WorkspacePathParam, "") + c.Set(shared.ConfigureMcpParam, true) + c.Set(shared.ConfigureRulesParam, false) // <- the gate the legacy cleanup must NOT respect + require.NoError(t, Configure(logger, c, uiStub, "/usr/local/bin/snyk")) + + // Dedicated rules file should NOT have been written (configureRules=false). + _, statErr := os.Stat(filepath.Join(home, ".claude", "rules", "snyk-security.md")) + assert.True(t, os.IsNotExist(statErr), "rules file must not be written when configureRules=false") + + // But the legacy block in CLAUDE.md MUST still be cleaned up. + md, err := os.ReadFile(claudeMd) + require.NoError(t, err) + assert.NotContains(t, string(md), RuleStart, "legacy cleanup must run even when configureRules=false") + assert.NotContains(t, string(md), "stale snyk") + assert.Contains(t, string(md), "# Personal") + }) +} diff --git a/internal/configure/host_config.go b/internal/configure/host_config.go index d423368..9a90ad6 100644 --- a/internal/configure/host_config.go +++ b/internal/configure/host_config.go @@ -8,12 +8,14 @@ import ( ) type hostConfig struct { - name string - mcpGlobalConfigPath string // Path to MCP server configuration JSON - localRulesPath string // Relative path for local workspace rules - globalRulesPath string // Absolute path for global user rules (delimited) - globalSkillsPath string // Absolute path for global user skills (no delimiters) - legacyLocalRulesPath string // Old local rules path to clean up during migration + name string + mcpGlobalConfigPath string // Path to MCP server configuration JSON + localRulesPath string // Relative path for local workspace rules + globalRulesPath string // Absolute path for global user rules (delimited block in a shared file) + globalSkillsPath string // Absolute path for a host-native skill file (e.g. Cursor SKILL.md, no delimiters) + globalDedicatedRulesPath string // Absolute path for a dedicated host-native rules file (e.g. Claude Code ~/.claude/rules/*.md, no delimiters) + legacyLocalRulesPath string // Old local rules path to clean up during migration (whole file removed) + legacyGlobalRulesPath string // Old global rules file to clean up during migration (delimited block removed in place; surrounding user content preserved) } // getHostConfig returns MCP-Host-specific configuration based on the host name @@ -69,10 +71,20 @@ func getHostConfig(hostName string) (*hostConfig, error) { globalRulesPath: filepath.Join(homeDir, ".gemini", "GEMINI.md"), }, nil case "claude-cli": + // Claude Code auto-loads any *.md file in ~/.claude/rules/ on every session + // (see https://code.claude.com/docs/en/memory#user-level-rules), so the rules + // belong in their own dedicated file rather than injected into the user's + // global ~/.claude/CLAUDE.md. globalDedicatedRulesPath is distinct from + // globalSkillsPath (Cursor's SKILL.md slot) so each host gets content + // formatted for its own loader ā€” Claude Code rules need no frontmatter, + // while Cursor's SKILL.md expects name/description keys. + // legacyGlobalRulesPath cleans up the old in-place injection from prior + // installs that wrote a delimited block into ~/.claude/CLAUDE.md. return &hostConfig{ - name: hostName, - mcpGlobalConfigPath: filepath.Join(homeDir, ".claude.json"), - globalRulesPath: filepath.Join(homeDir, ".claude", "CLAUDE.md"), + name: hostName, + mcpGlobalConfigPath: filepath.Join(homeDir, ".claude.json"), + globalDedicatedRulesPath: filepath.Join(homeDir, ".claude", "rules", "snyk-security.md"), + legacyGlobalRulesPath: filepath.Join(homeDir, ".claude", "CLAUDE.md"), }, nil } return nil, fmt.Errorf("unsupported Tool: %s", hostName) diff --git a/internal/configure/rules.go b/internal/configure/rules.go index 56575ea..c729dd3 100644 --- a/internal/configure/rules.go +++ b/internal/configure/rules.go @@ -154,22 +154,54 @@ func removeGlobalRules(targetFile string, logger *zerolog.Logger) error { return nil } -// removeDelimitedBlock removes a delimited block from the content +// findDelimitedBlockLines locates a delimited block in a normalized source +// using line-anchored matching: both markers must appear on their own line +// (no surrounding text on the same line), which matches what +// writeGlobalRules / writeGlobalSkills emit. This avoids destroying user +// content that quotes the marker text inline (e.g. in a how-to doc) and +// is robust against multiple complete blocks: scanning from the end +// returns the LAST start that has a matching end after it. +// +// Returns the (startLine, endLine) zero-based line indices, or (-1, -1) +// when no complete delimited pair is found. +func findDelimitedBlockLines(lines []string, start, end string) (int, int) { + for i := len(lines) - 1; i >= 0; i-- { + if lines[i] != start { + continue + } + for j := i + 1; j < len(lines); j++ { + if lines[j] == end { + return i, j + } + } + } + return -1, -1 +} + +// removeDelimitedBlock removes a delimited block from the content. Matching +// is line-anchored (see findDelimitedBlockLines); a user's CLAUDE.md that +// quotes the literal marker text inline within a sentence is preserved. func removeDelimitedBlock(source, start, end string) string { // Normalize newlines to \n src := strings.ReplaceAll(source, "\r\n", "\n") + lines := strings.Split(src, "\n") - startIdx := strings.Index(src, start) - endIdx := strings.Index(src, end) - - if startIdx == -1 || endIdx == -1 || endIdx <= startIdx { - // No block found + startLine, endLine := findDelimitedBlockLines(lines, start, end) + if startLine == -1 { + // No complete delimited pair found; leave source untouched. This + // returns the ORIGINAL source (not normalized) so a CRLF file with + // no Snyk block survives byte-for-byte. return source } - // Remove from start marker to end marker (inclusive of the end marker) - before := trimTrailingNewlines(src[:startIdx]) - after := trimLeadingNewlines(src[endIdx+len(end):]) + before := strings.Join(lines[:startLine], "\n") + after := "" + if endLine+1 < len(lines) { + after = strings.Join(lines[endLine+1:], "\n") + } + + before = trimTrailingNewlines(before) + after = trimLeadingNewlines(after) separator := "" if before != "" && after != "" { @@ -185,18 +217,22 @@ func removeDelimitedBlock(source, start, end string) string { return result } -// upsertDelimitedBlock replaces or appends a delimited block inside a file content +// upsertDelimitedBlock replaces or appends a delimited block inside a file +// content. Matching is line-anchored and uses the same robust pair-finding +// rule as removeDelimitedBlock ā€” see findDelimitedBlockLines. func upsertDelimitedBlock(source, start, end, fullBlockToInsert string) string { // Normalize newlines to \n src := strings.ReplaceAll(source, "\r\n", "\n") - - startIdx := strings.Index(src, start) - endIdx := strings.Index(src, end) - - if startIdx != -1 && endIdx != -1 && endIdx > startIdx { - // Replace from start marker to end marker (inclusive) - before := src[:startIdx] - after := src[endIdx+len(end):] + lines := strings.Split(src, "\n") + + startLine, endLine := findDelimitedBlockLines(lines, start, end) + if startLine != -1 { + // Replace from startLine to endLine (inclusive) + before := strings.Join(lines[:startLine], "\n") + after := "" + if endLine+1 < len(lines) { + after = strings.Join(lines[endLine+1:], "\n") + } return trimTrailingNewlines(before) + "\n" + strings.TrimSpace(fullBlockToInsert) + "\n" + trimLeadingNewlines(after) } diff --git a/internal/configure/rules/claude/always_apply.md b/internal/configure/rules/claude/always_apply.md new file mode 100644 index 0000000..dbbc396 --- /dev/null +++ b/internal/configure/rules/claude/always_apply.md @@ -0,0 +1,8 @@ +# Snyk Security At Inception + +Project security best practices for AI-generated code: + +- Always run snyk_code_scan tool for new first party code that is generated in a Snyk-supported language. +- If any security issues are found based on newly introduced or modified code or dependencies, attempt to fix the issues using the results context from Snyk. +- Rescan the code after fixing the issues to ensure that the issues were fixed and that there are no newly introduced issues. +- Repeat this process until no new issues are found. diff --git a/internal/configure/rules/claude/smart_apply.md b/internal/configure/rules/claude/smart_apply.md new file mode 100644 index 0000000..13b1c66 --- /dev/null +++ b/internal/configure/rules/claude/smart_apply.md @@ -0,0 +1,9 @@ +# Snyk Security At Inception + +Project security best practices for AI-generated code: + +- BEFORE declaring task complete: Run snyk_code_scan tool when a significant change has been made in first party code. +- This should only apply for Snyk-supported coding language +- If any security issues are found based on newly introduced or modified code or dependencies, attempt to fix the issues using the results context from Snyk. +- Rescan the code after fixing the issues to ensure that the issues were fixed and that there are no newly introduced issues. +- Repeat this process until no new issues are found.