From cd921174f47afc60c2ee1cd5990b00fc7e66bbac Mon Sep 17 00:00:00 2001 From: Mike North Date: Sun, 15 Mar 2026 14:27:21 -0700 Subject: [PATCH 1/3] Add sentinel block management and manual remnant detection Adds reusable utilities for safely managing a delimited block in shell config files (e.g., ~/.zshrc, ~/.bashrc). These are the building blocks for the upcoming `stripe completion --install` feature. - addSentinelBlock: inserts or replaces a sentinel-delimited block - removeSentinelBlock: removes the sentinel-delimited block - findManualRemnants: scans for completion references outside the managed block (e.g., manually-added source lines) Handles edge cases: missing files, orphaned markers, reversed markers, files without trailing newlines. Preserves existing file permissions. Committed-By-Agent: claude --- pkg/cmd/completion.go | 152 +++++++++++++ pkg/cmd/completion_test.go | 451 +++++++++++++++++++++++++++++++++++++ 2 files changed, 603 insertions(+) diff --git a/pkg/cmd/completion.go b/pkg/cmd/completion.go index 501416e9..29a1cb78 100644 --- a/pkg/cmd/completion.go +++ b/pkg/cmd/completion.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "github.com/spf13/cobra" @@ -195,3 +196,154 @@ func detectShell() string { return "" } } + +// sentinelBegin and sentinelEnd mark the completion configuration block +// in shell config files (~/.zshrc, ~/.bashrc, ~/.bash_profile). This allows +// safe idempotent install/uninstall without corrupting the user's existing config. +const ( + sentinelBegin = "# begin stripe-completion — managed by stripe cli, do not edit" + sentinelEnd = "# end stripe-completion" +) + +// addSentinelBlock adds or replaces a sentinel-delimited block in the given +// config file. If the file does not exist, it is created with mode 0644. +// Existing file permissions are preserved. The operation is idempotent: +// calling it twice with the same line produces the same result as calling +// it once. If the file contains orphaned or reversed markers, a new block +// is appended rather than attempting to repair the malformed state. +func addSentinelBlock(configPath, line string) error { + block := fmt.Sprintf("%s\n%s\n%s", sentinelBegin, line, sentinelEnd) + + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + // Create new file with just the sentinel block + return os.WriteFile(configPath, []byte(block+"\n"), 0644) + } + return err + } + + // Preserve existing file permissions + perm := os.FileMode(0644) + if info, statErr := os.Stat(configPath); statErr == nil { + perm = info.Mode().Perm() + } + + content := string(data) + + // Replace existing block if both markers are present in the correct order. + // Orphaned or reversed markers are left untouched — we append instead. + beginIdx := strings.Index(content, sentinelBegin) + endIdx := strings.Index(content, sentinelEnd) + if beginIdx >= 0 && endIdx >= 0 && endIdx > beginIdx { + endIdx += len(sentinelEnd) + // Include trailing newline if present + if endIdx < len(content) && content[endIdx] == '\n' { + endIdx++ + } + content = content[:beginIdx] + block + "\n" + content[endIdx:] + return os.WriteFile(configPath, []byte(content), perm) + } + + // Append sentinel block + if len(content) > 0 && !strings.HasSuffix(content, "\n") { + content += "\n" + } + content += block + "\n" + + return os.WriteFile(configPath, []byte(content), perm) +} + +// removeSentinelBlock removes the sentinel-delimited block from the given +// config file. If the file does not exist, this is a no-op. If the markers +// are orphaned or reversed, the file is left unchanged. Existing file +// permissions are preserved. +func removeSentinelBlock(configPath string) error { + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + + content := string(data) + + beginIdx := strings.Index(content, sentinelBegin) + endIdx := strings.Index(content, sentinelEnd) + if beginIdx < 0 || endIdx < 0 || endIdx <= beginIdx { + // No valid sentinel block found, nothing to do + return nil + } + + // Preserve existing file permissions + perm := os.FileMode(0644) + if info, statErr := os.Stat(configPath); statErr == nil { + perm = info.Mode().Perm() + } + + endIdx += len(sentinelEnd) + // Include trailing newline if present + if endIdx < len(content) && content[endIdx] == '\n' { + endIdx++ + } + + content = content[:beginIdx] + content[endIdx:] + + return os.WriteFile(configPath, []byte(content), perm) +} + +// manualRemnant represents a line in a shell config file that references the +// completion script but is outside our sentinel-managed block. +type manualRemnant struct { + lineNumber int // 1-based, for display in user-facing warnings + lineText string // trimmed content of the matching line +} + +// findManualRemnants scans a shell config file for lines referencing the +// completion script filename that are outside our sentinel block. This detects +// manually-added source/load lines that the user may need to clean up. +// +// Lines inside the sentinel block, blank lines, and comment lines (starting +// with #) are excluded from the scan. Returns nil if the file cannot be read +// or no matches are found. +func findManualRemnants(configPath, scriptFilename string) []manualRemnant { + data, err := os.ReadFile(configPath) + if err != nil { + return nil + } + + var remnants []manualRemnant + inSentinelBlock := false + + for i, line := range strings.Split(string(data), "\n") { + trimmed := strings.TrimSpace(line) + + if trimmed == sentinelBegin { + inSentinelBlock = true + continue + } + if trimmed == sentinelEnd { + inSentinelBlock = false + continue + } + + if inSentinelBlock { + continue + } + + // Skip blank lines and comments + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + if strings.Contains(trimmed, scriptFilename) { + remnants = append(remnants, manualRemnant{ + lineNumber: i + 1, + lineText: trimmed, + }) + } + } + + return remnants +} diff --git a/pkg/cmd/completion_test.go b/pkg/cmd/completion_test.go index 2fc9cb78..f9dca993 100644 --- a/pkg/cmd/completion_test.go +++ b/pkg/cmd/completion_test.go @@ -1,13 +1,21 @@ package cmd import ( + "fmt" "os" + "path/filepath" + "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// --------------------------------------------------------------------------- +// Tests from fish-completion branch (preserved) +// --------------------------------------------------------------------------- + func TestDetectShell(t *testing.T) { tests := []struct { name string @@ -164,3 +172,446 @@ func TestGenShellCreatesFile(t *testing.T) { }) } } + +// --------------------------------------------------------------------------- +// addSentinelBlock / removeSentinelBlock +// --------------------------------------------------------------------------- + +func TestAddSentinelBlockToNewFile(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + err := addSentinelBlock(configPath, "source /home/user/.stripe/stripe-completion.zsh") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + content := string(data) + assert.Contains(t, content, sentinelBegin) + assert.Contains(t, content, "source /home/user/.stripe/stripe-completion.zsh") + assert.Contains(t, content, sentinelEnd) +} + +func TestAddSentinelBlockPreservesExistingContent(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + existing := "export PATH=/usr/local/bin:$PATH\nalias ll='ls -la'\n" + require.NoError(t, os.WriteFile(configPath, []byte(existing), 0644)) + + err := addSentinelBlock(configPath, "source /home/user/.stripe/stripe-completion.zsh") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + content := string(data) + assert.True(t, strings.HasPrefix(content, existing), "existing content should be preserved at the start") + assert.Contains(t, content, sentinelBegin) + assert.Contains(t, content, sentinelEnd) +} + +func TestAddSentinelBlockReplaceExisting(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + initial := fmt.Sprintf("before\n%s\nold source line\n%s\nafter\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(initial), 0644)) + + err := addSentinelBlock(configPath, "new source line") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + content := string(data) + assert.Contains(t, content, "before\n") + assert.Contains(t, content, "new source line") + assert.NotContains(t, content, "old source line") + assert.Contains(t, content, "after\n") + + assert.Equal(t, 1, strings.Count(content, sentinelBegin)) + assert.Equal(t, 1, strings.Count(content, sentinelEnd)) +} + +func TestAddSentinelBlockAppendsNewlineIfMissing(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + require.NoError(t, os.WriteFile(configPath, []byte("no trailing newline"), 0644)) + + err := addSentinelBlock(configPath, "source line") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + content := string(data) + assert.True(t, strings.Contains(content, "no trailing newline\n"+sentinelBegin)) +} + +func TestAddSentinelBlockOrphanedBeginOnly(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\norphaned source line\nafter\n", sentinelBegin) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := addSentinelBlock(configPath, "new source line") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + result := string(data) + assert.Contains(t, result, "new source line") + assert.Contains(t, result, sentinelEnd) +} + +func TestAddSentinelBlockOrphanedEndOnly(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\nafter\n", sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := addSentinelBlock(configPath, "new source line") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + result := string(data) + assert.Contains(t, result, "new source line") + assert.Equal(t, 1, strings.Count(result, sentinelBegin), "should have exactly one begin marker") + assert.GreaterOrEqual(t, strings.Count(result, sentinelEnd), 1, "should have at least one end marker") +} + +func TestAddSentinelBlockReversedMarkers(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := addSentinelBlock(configPath, "new source line") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + result := string(data) + assert.Contains(t, result, "new source line") +} + +func TestRemoveSentinelBlockPreservesOtherContent(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\nsource line\n%s\nafter\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + result := string(data) + assert.Contains(t, result, "before\n") + assert.Contains(t, result, "after\n") + assert.NotContains(t, result, sentinelBegin) + assert.NotContains(t, result, sentinelEnd) + assert.NotContains(t, result, "source line") +} + +func TestRemoveSentinelBlockNoBlockPresent(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + original := "export FOO=bar\n" + require.NoError(t, os.WriteFile(configPath, []byte(original), 0644)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, original, string(data)) +} + +func TestRemoveSentinelBlockFileMissing(t *testing.T) { + err := removeSentinelBlock(filepath.Join(t.TempDir(), "nonexistent")) + assert.NoError(t, err) +} + +func TestRemoveSentinelBlockOrphanedBeginOnly(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\norphaned\nafter\n", sentinelBegin) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, content, string(data)) +} + +func TestRemoveSentinelBlockOrphanedEndOnly(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\nafter\n", sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, content, string(data)) +} + +func TestRemoveSentinelBlockReversedMarkers(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + // End marker appears before begin — should be a no-op + content := fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, content, string(data), "reversed markers should be left untouched") +} + +func TestAddSentinelBlockReadPermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("content"), 0644)) + require.NoError(t, os.Chmod(configPath, 0000)) + t.Cleanup(func() { os.Chmod(configPath, 0644) }) + + err := addSentinelBlock(configPath, "source line") + assert.Error(t, err) +} + +func TestAddSentinelBlockWritePermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0644)) + require.NoError(t, os.Chmod(configPath, 0444)) + t.Cleanup(func() { os.Chmod(configPath, 0644) }) + + err := addSentinelBlock(configPath, "source line") + assert.Error(t, err) +} + +func TestRemoveSentinelBlockReadPermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + content := fmt.Sprintf("%s\nline\n%s\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + require.NoError(t, os.Chmod(configPath, 0000)) + t.Cleanup(func() { os.Chmod(configPath, 0644) }) + + err := removeSentinelBlock(configPath) + assert.Error(t, err) +} + +func TestRemoveSentinelBlockWritePermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + content := fmt.Sprintf("%s\nline\n%s\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + require.NoError(t, os.Chmod(configPath, 0444)) + t.Cleanup(func() { os.Chmod(configPath, 0644) }) + + err := removeSentinelBlock(configPath) + assert.Error(t, err) +} + +func TestAddSentinelBlockPreservesFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0600)) + + err := addSentinelBlock(configPath, "source line") + require.NoError(t, err) + + info, err := os.Stat(configPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") +} + +func TestRemoveSentinelBlockPreservesFilePermissions(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + content := fmt.Sprintf("before\n%s\nline\n%s\nafter\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0600)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + info, err := os.Stat(configPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") +} + +// --------------------------------------------------------------------------- +// findManualRemnants +// --------------------------------------------------------------------------- + +func TestFindManualRemnantsDetectsManualSourceLine(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "export PATH=/usr/local/bin:$PATH\nsource ~/.stripe/stripe-completion.zsh\nalias ls='ls -G'\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 1) + assert.Equal(t, 2, remnants[0].lineNumber) + assert.Contains(t, remnants[0].lineText, "stripe-completion.zsh") +} + +func TestFindManualRemnantsDetectsDotSourceSyntax(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".bashrc") + + content := ". /some/custom/path/stripe-completion.bash\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.bash") + require.Len(t, remnants, 1) + assert.Equal(t, 1, remnants[0].lineNumber) +} + +func TestFindManualRemnantsDetectsLineWithOtherCommands(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "[ -f ~/.stripe/stripe-completion.zsh ] && source ~/.stripe/stripe-completion.zsh\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 1) + assert.Equal(t, 1, remnants[0].lineNumber) +} + +func TestFindManualRemnantsDetectsCustomPath(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "source /opt/completions/stripe-completion.zsh\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 1) +} + +func TestFindManualRemnantsIgnoresSentinelBlock(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("before\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nafter\n", + sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + assert.Empty(t, remnants) +} + +func TestFindManualRemnantsIgnoresComments(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "# source ~/.stripe/stripe-completion.zsh\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + assert.Empty(t, remnants) +} + +func TestFindManualRemnantsReturnsNilForMissingFile(t *testing.T) { + remnants := findManualRemnants(filepath.Join(t.TempDir(), "nonexistent"), "stripe-completion.zsh") + assert.Nil(t, remnants) +} + +func TestFindManualRemnantsNoMatchReturnsNil(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "export PATH=/usr/local/bin:$PATH\nalias ls='ls -G'\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + assert.Nil(t, remnants) +} + +func TestFindManualRemnantsMultipleMatches(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := "source ~/.stripe/stripe-completion.zsh\nexport FOO=bar\n. /other/stripe-completion.zsh\n" + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 2) + assert.Equal(t, 1, remnants[0].lineNumber) + assert.Equal(t, 3, remnants[1].lineNumber) +} + +func TestFindManualRemnantsOutsideSentinelWithManualBefore(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("source ~/my/stripe-completion.zsh\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\n", + sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 1) + assert.Equal(t, 1, remnants[0].lineNumber) +} + +func TestFindManualRemnantsOutsideSentinelWithManualAfter(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + content := fmt.Sprintf("%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nsource ~/custom/stripe-completion.zsh\n", + sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) + + remnants := findManualRemnants(configPath, "stripe-completion.zsh") + require.Len(t, remnants, 1) + assert.Equal(t, 4, remnants[0].lineNumber) +} From d1df7aef3399cea2a96f3998375ba26e9497b91b Mon Sep 17 00:00:00 2001 From: Mike North Date: Tue, 14 Apr 2026 11:35:17 -0700 Subject: [PATCH 2/3] refactor: extract sentinel block management with pure functions and atomic writes Move sentinel block logic from completion.go into a dedicated sentinel.go. Extract pure text-transformation functions (computeAddSentinel, computeRemoveSentinel) from I/O wrappers, add atomic file writes via a temp-file-then-rename approach, use TOCTOU-safe permission reads, switch sentinel markers to ASCII, and use substring matching in findManualRemnants for consistency. Tests move to sentinel_test.go with table-driven pure function coverage and updated I/O tests that reflect atomic write semantics. Committed-By-Agent: claude --- pkg/cmd/completion.go | 152 ------------- pkg/cmd/completion_test.go | 447 ------------------------------------- pkg/cmd/sentinel.go | 221 ++++++++++++++++++ pkg/cmd/sentinel_test.go | 352 +++++++++++++++++++++++++++++ 4 files changed, 573 insertions(+), 599 deletions(-) create mode 100644 pkg/cmd/sentinel.go create mode 100644 pkg/cmd/sentinel_test.go diff --git a/pkg/cmd/completion.go b/pkg/cmd/completion.go index 29a1cb78..501416e9 100644 --- a/pkg/cmd/completion.go +++ b/pkg/cmd/completion.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "github.com/spf13/cobra" @@ -196,154 +195,3 @@ func detectShell() string { return "" } } - -// sentinelBegin and sentinelEnd mark the completion configuration block -// in shell config files (~/.zshrc, ~/.bashrc, ~/.bash_profile). This allows -// safe idempotent install/uninstall without corrupting the user's existing config. -const ( - sentinelBegin = "# begin stripe-completion — managed by stripe cli, do not edit" - sentinelEnd = "# end stripe-completion" -) - -// addSentinelBlock adds or replaces a sentinel-delimited block in the given -// config file. If the file does not exist, it is created with mode 0644. -// Existing file permissions are preserved. The operation is idempotent: -// calling it twice with the same line produces the same result as calling -// it once. If the file contains orphaned or reversed markers, a new block -// is appended rather than attempting to repair the malformed state. -func addSentinelBlock(configPath, line string) error { - block := fmt.Sprintf("%s\n%s\n%s", sentinelBegin, line, sentinelEnd) - - data, err := os.ReadFile(configPath) - if err != nil { - if os.IsNotExist(err) { - // Create new file with just the sentinel block - return os.WriteFile(configPath, []byte(block+"\n"), 0644) - } - return err - } - - // Preserve existing file permissions - perm := os.FileMode(0644) - if info, statErr := os.Stat(configPath); statErr == nil { - perm = info.Mode().Perm() - } - - content := string(data) - - // Replace existing block if both markers are present in the correct order. - // Orphaned or reversed markers are left untouched — we append instead. - beginIdx := strings.Index(content, sentinelBegin) - endIdx := strings.Index(content, sentinelEnd) - if beginIdx >= 0 && endIdx >= 0 && endIdx > beginIdx { - endIdx += len(sentinelEnd) - // Include trailing newline if present - if endIdx < len(content) && content[endIdx] == '\n' { - endIdx++ - } - content = content[:beginIdx] + block + "\n" + content[endIdx:] - return os.WriteFile(configPath, []byte(content), perm) - } - - // Append sentinel block - if len(content) > 0 && !strings.HasSuffix(content, "\n") { - content += "\n" - } - content += block + "\n" - - return os.WriteFile(configPath, []byte(content), perm) -} - -// removeSentinelBlock removes the sentinel-delimited block from the given -// config file. If the file does not exist, this is a no-op. If the markers -// are orphaned or reversed, the file is left unchanged. Existing file -// permissions are preserved. -func removeSentinelBlock(configPath string) error { - data, err := os.ReadFile(configPath) - if err != nil { - if os.IsNotExist(err) { - return nil - } - return err - } - - content := string(data) - - beginIdx := strings.Index(content, sentinelBegin) - endIdx := strings.Index(content, sentinelEnd) - if beginIdx < 0 || endIdx < 0 || endIdx <= beginIdx { - // No valid sentinel block found, nothing to do - return nil - } - - // Preserve existing file permissions - perm := os.FileMode(0644) - if info, statErr := os.Stat(configPath); statErr == nil { - perm = info.Mode().Perm() - } - - endIdx += len(sentinelEnd) - // Include trailing newline if present - if endIdx < len(content) && content[endIdx] == '\n' { - endIdx++ - } - - content = content[:beginIdx] + content[endIdx:] - - return os.WriteFile(configPath, []byte(content), perm) -} - -// manualRemnant represents a line in a shell config file that references the -// completion script but is outside our sentinel-managed block. -type manualRemnant struct { - lineNumber int // 1-based, for display in user-facing warnings - lineText string // trimmed content of the matching line -} - -// findManualRemnants scans a shell config file for lines referencing the -// completion script filename that are outside our sentinel block. This detects -// manually-added source/load lines that the user may need to clean up. -// -// Lines inside the sentinel block, blank lines, and comment lines (starting -// with #) are excluded from the scan. Returns nil if the file cannot be read -// or no matches are found. -func findManualRemnants(configPath, scriptFilename string) []manualRemnant { - data, err := os.ReadFile(configPath) - if err != nil { - return nil - } - - var remnants []manualRemnant - inSentinelBlock := false - - for i, line := range strings.Split(string(data), "\n") { - trimmed := strings.TrimSpace(line) - - if trimmed == sentinelBegin { - inSentinelBlock = true - continue - } - if trimmed == sentinelEnd { - inSentinelBlock = false - continue - } - - if inSentinelBlock { - continue - } - - // Skip blank lines and comments - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - - if strings.Contains(trimmed, scriptFilename) { - remnants = append(remnants, manualRemnant{ - lineNumber: i + 1, - lineText: trimmed, - }) - } - } - - return remnants -} diff --git a/pkg/cmd/completion_test.go b/pkg/cmd/completion_test.go index f9dca993..e6ee0e15 100644 --- a/pkg/cmd/completion_test.go +++ b/pkg/cmd/completion_test.go @@ -1,11 +1,7 @@ package cmd import ( - "fmt" "os" - "path/filepath" - "runtime" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -172,446 +168,3 @@ func TestGenShellCreatesFile(t *testing.T) { }) } } - -// --------------------------------------------------------------------------- -// addSentinelBlock / removeSentinelBlock -// --------------------------------------------------------------------------- - -func TestAddSentinelBlockToNewFile(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - err := addSentinelBlock(configPath, "source /home/user/.stripe/stripe-completion.zsh") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - - content := string(data) - assert.Contains(t, content, sentinelBegin) - assert.Contains(t, content, "source /home/user/.stripe/stripe-completion.zsh") - assert.Contains(t, content, sentinelEnd) -} - -func TestAddSentinelBlockPreservesExistingContent(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - existing := "export PATH=/usr/local/bin:$PATH\nalias ll='ls -la'\n" - require.NoError(t, os.WriteFile(configPath, []byte(existing), 0644)) - - err := addSentinelBlock(configPath, "source /home/user/.stripe/stripe-completion.zsh") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - - content := string(data) - assert.True(t, strings.HasPrefix(content, existing), "existing content should be preserved at the start") - assert.Contains(t, content, sentinelBegin) - assert.Contains(t, content, sentinelEnd) -} - -func TestAddSentinelBlockReplaceExisting(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - initial := fmt.Sprintf("before\n%s\nold source line\n%s\nafter\n", sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(initial), 0644)) - - err := addSentinelBlock(configPath, "new source line") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - - content := string(data) - assert.Contains(t, content, "before\n") - assert.Contains(t, content, "new source line") - assert.NotContains(t, content, "old source line") - assert.Contains(t, content, "after\n") - - assert.Equal(t, 1, strings.Count(content, sentinelBegin)) - assert.Equal(t, 1, strings.Count(content, sentinelEnd)) -} - -func TestAddSentinelBlockAppendsNewlineIfMissing(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - require.NoError(t, os.WriteFile(configPath, []byte("no trailing newline"), 0644)) - - err := addSentinelBlock(configPath, "source line") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - - content := string(data) - assert.True(t, strings.Contains(content, "no trailing newline\n"+sentinelBegin)) -} - -func TestAddSentinelBlockOrphanedBeginOnly(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\norphaned source line\nafter\n", sentinelBegin) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := addSentinelBlock(configPath, "new source line") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - result := string(data) - assert.Contains(t, result, "new source line") - assert.Contains(t, result, sentinelEnd) -} - -func TestAddSentinelBlockOrphanedEndOnly(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\nafter\n", sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := addSentinelBlock(configPath, "new source line") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - result := string(data) - assert.Contains(t, result, "new source line") - assert.Equal(t, 1, strings.Count(result, sentinelBegin), "should have exactly one begin marker") - assert.GreaterOrEqual(t, strings.Count(result, sentinelEnd), 1, "should have at least one end marker") -} - -func TestAddSentinelBlockReversedMarkers(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := addSentinelBlock(configPath, "new source line") - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - result := string(data) - assert.Contains(t, result, "new source line") -} - -func TestRemoveSentinelBlockPreservesOtherContent(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\nsource line\n%s\nafter\n", sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - - result := string(data) - assert.Contains(t, result, "before\n") - assert.Contains(t, result, "after\n") - assert.NotContains(t, result, sentinelBegin) - assert.NotContains(t, result, sentinelEnd) - assert.NotContains(t, result, "source line") -} - -func TestRemoveSentinelBlockNoBlockPresent(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - original := "export FOO=bar\n" - require.NoError(t, os.WriteFile(configPath, []byte(original), 0644)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - assert.Equal(t, original, string(data)) -} - -func TestRemoveSentinelBlockFileMissing(t *testing.T) { - err := removeSentinelBlock(filepath.Join(t.TempDir(), "nonexistent")) - assert.NoError(t, err) -} - -func TestRemoveSentinelBlockOrphanedBeginOnly(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\norphaned\nafter\n", sentinelBegin) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - assert.Equal(t, content, string(data)) -} - -func TestRemoveSentinelBlockOrphanedEndOnly(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\nafter\n", sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - assert.Equal(t, content, string(data)) -} - -func TestRemoveSentinelBlockReversedMarkers(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - // End marker appears before begin — should be a no-op - content := fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - data, err := os.ReadFile(configPath) - require.NoError(t, err) - assert.Equal(t, content, string(data), "reversed markers should be left untouched") -} - -func TestAddSentinelBlockReadPermissionDenied(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - require.NoError(t, os.WriteFile(configPath, []byte("content"), 0644)) - require.NoError(t, os.Chmod(configPath, 0000)) - t.Cleanup(func() { os.Chmod(configPath, 0644) }) - - err := addSentinelBlock(configPath, "source line") - assert.Error(t, err) -} - -func TestAddSentinelBlockWritePermissionDenied(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0644)) - require.NoError(t, os.Chmod(configPath, 0444)) - t.Cleanup(func() { os.Chmod(configPath, 0644) }) - - err := addSentinelBlock(configPath, "source line") - assert.Error(t, err) -} - -func TestRemoveSentinelBlockReadPermissionDenied(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - content := fmt.Sprintf("%s\nline\n%s\n", sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - require.NoError(t, os.Chmod(configPath, 0000)) - t.Cleanup(func() { os.Chmod(configPath, 0644) }) - - err := removeSentinelBlock(configPath) - assert.Error(t, err) -} - -func TestRemoveSentinelBlockWritePermissionDenied(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - content := fmt.Sprintf("%s\nline\n%s\n", sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - require.NoError(t, os.Chmod(configPath, 0444)) - t.Cleanup(func() { os.Chmod(configPath, 0644) }) - - err := removeSentinelBlock(configPath) - assert.Error(t, err) -} - -func TestAddSentinelBlockPreservesFilePermissions(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0600)) - - err := addSentinelBlock(configPath, "source line") - require.NoError(t, err) - - info, err := os.Stat(configPath) - require.NoError(t, err) - assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") -} - -func TestRemoveSentinelBlockPreservesFilePermissions(t *testing.T) { - if runtime.GOOS == "windows" || os.Getuid() == 0 { - t.Skip("Cannot test Unix file permissions on Windows or as root") - } - - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - content := fmt.Sprintf("before\n%s\nline\n%s\nafter\n", sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0600)) - - err := removeSentinelBlock(configPath) - require.NoError(t, err) - - info, err := os.Stat(configPath) - require.NoError(t, err) - assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") -} - -// --------------------------------------------------------------------------- -// findManualRemnants -// --------------------------------------------------------------------------- - -func TestFindManualRemnantsDetectsManualSourceLine(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "export PATH=/usr/local/bin:$PATH\nsource ~/.stripe/stripe-completion.zsh\nalias ls='ls -G'\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 1) - assert.Equal(t, 2, remnants[0].lineNumber) - assert.Contains(t, remnants[0].lineText, "stripe-completion.zsh") -} - -func TestFindManualRemnantsDetectsDotSourceSyntax(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".bashrc") - - content := ". /some/custom/path/stripe-completion.bash\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.bash") - require.Len(t, remnants, 1) - assert.Equal(t, 1, remnants[0].lineNumber) -} - -func TestFindManualRemnantsDetectsLineWithOtherCommands(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "[ -f ~/.stripe/stripe-completion.zsh ] && source ~/.stripe/stripe-completion.zsh\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 1) - assert.Equal(t, 1, remnants[0].lineNumber) -} - -func TestFindManualRemnantsDetectsCustomPath(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "source /opt/completions/stripe-completion.zsh\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 1) -} - -func TestFindManualRemnantsIgnoresSentinelBlock(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("before\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nafter\n", - sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - assert.Empty(t, remnants) -} - -func TestFindManualRemnantsIgnoresComments(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "# source ~/.stripe/stripe-completion.zsh\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - assert.Empty(t, remnants) -} - -func TestFindManualRemnantsReturnsNilForMissingFile(t *testing.T) { - remnants := findManualRemnants(filepath.Join(t.TempDir(), "nonexistent"), "stripe-completion.zsh") - assert.Nil(t, remnants) -} - -func TestFindManualRemnantsNoMatchReturnsNil(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "export PATH=/usr/local/bin:$PATH\nalias ls='ls -G'\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - assert.Nil(t, remnants) -} - -func TestFindManualRemnantsMultipleMatches(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := "source ~/.stripe/stripe-completion.zsh\nexport FOO=bar\n. /other/stripe-completion.zsh\n" - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 2) - assert.Equal(t, 1, remnants[0].lineNumber) - assert.Equal(t, 3, remnants[1].lineNumber) -} - -func TestFindManualRemnantsOutsideSentinelWithManualBefore(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("source ~/my/stripe-completion.zsh\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\n", - sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 1) - assert.Equal(t, 1, remnants[0].lineNumber) -} - -func TestFindManualRemnantsOutsideSentinelWithManualAfter(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".zshrc") - - content := fmt.Sprintf("%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nsource ~/custom/stripe-completion.zsh\n", - sentinelBegin, sentinelEnd) - require.NoError(t, os.WriteFile(configPath, []byte(content), 0644)) - - remnants := findManualRemnants(configPath, "stripe-completion.zsh") - require.Len(t, remnants, 1) - assert.Equal(t, 4, remnants[0].lineNumber) -} diff --git a/pkg/cmd/sentinel.go b/pkg/cmd/sentinel.go new file mode 100644 index 00000000..89a8f695 --- /dev/null +++ b/pkg/cmd/sentinel.go @@ -0,0 +1,221 @@ +package cmd + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +// sentinelBegin and sentinelEnd mark the completion configuration block +// in shell config files (~/.zshrc, ~/.bashrc, ~/.bash_profile). This allows +// safe idempotent install/uninstall without corrupting the user's existing config. +const ( + sentinelBegin = "# begin stripe-completion -- managed by stripe cli, do not edit" + sentinelEnd = "# end stripe-completion" +) + +// computeAddSentinel returns the new file content with the sentinel block added +// or replaced. It performs no I/O. If both markers are present in the correct +// order, the existing block is replaced. If markers are absent, orphaned, or +// reversed, a new block is appended to the content. +func computeAddSentinel(content, line string) string { + block := fmt.Sprintf("%s\n%s\n%s", sentinelBegin, line, sentinelEnd) + + beginIdx := strings.Index(content, sentinelBegin) + endIdx := strings.Index(content, sentinelEnd) + if beginIdx >= 0 && endIdx >= 0 && endIdx > beginIdx { + end := endIdx + len(sentinelEnd) + // Include trailing newline if present + if end < len(content) && content[end] == '\n' { + end++ + } + return content[:beginIdx] + block + "\n" + content[end:] + } + + // Append sentinel block + if len(content) > 0 && !strings.HasSuffix(content, "\n") { + content += "\n" + } + return content + block + "\n" +} + +// computeRemoveSentinel returns the new file content with the sentinel block +// removed and a boolean indicating whether a block was found and removed. It +// performs no I/O. If markers are absent, orphaned, or reversed, the content +// is returned unchanged with false. +func computeRemoveSentinel(content string) (string, bool) { + beginIdx := strings.Index(content, sentinelBegin) + endIdx := strings.Index(content, sentinelEnd) + if beginIdx < 0 || endIdx < 0 || endIdx <= beginIdx { + return content, false + } + + end := endIdx + len(sentinelEnd) + // Include trailing newline if present + if end < len(content) && content[end] == '\n' { + end++ + } + return content[:beginIdx] + content[end:], true +} + +// readConfigFile opens the file at path, reads its contents, and returns the +// content string along with the file's permission bits. The file is opened once +// and stat'd on the same file descriptor to avoid TOCTOU races. If the file +// does not exist, ("", 0644, nil) is returned. +func readConfigFile(path string) (string, os.FileMode, error) { + f, err := os.Open(path) + if err != nil { + if os.IsNotExist(err) { + return "", 0644, nil + } + return "", 0, fmt.Errorf("reading %s: %w", path, err) + } + defer f.Close() + + info, err := f.Stat() + if err != nil { + return "", 0, fmt.Errorf("reading %s: %w", path, err) + } + perm := info.Mode().Perm() + + data, err := io.ReadAll(f) + if err != nil { + return "", 0, fmt.Errorf("reading %s: %w", path, err) + } + + return string(data), perm, nil +} + +// atomicWriteFile writes data to path atomically by creating a temporary file +// in the same directory, syncing, and renaming over the destination. This +// avoids partial writes visible to concurrent readers. On any error after the +// temp file is created, the temp file is removed. +func atomicWriteFile(path string, data []byte, perm os.FileMode) error { + dir := filepath.Dir(path) + tmp, err := os.CreateTemp(dir, ".stripe-*") + if err != nil { + return fmt.Errorf("writing %s: %w", path, err) + } + tmpName := tmp.Name() + + // Ensure cleanup on any error path after file creation. + var writeErr error + defer func() { + if writeErr != nil { + os.Remove(tmpName) + } + }() + + if _, writeErr = tmp.Write(data); writeErr != nil { + tmp.Close() + return fmt.Errorf("writing %s: %w", path, writeErr) + } + if writeErr = tmp.Sync(); writeErr != nil { + tmp.Close() + return fmt.Errorf("writing %s: %w", path, writeErr) + } + if writeErr = tmp.Close(); writeErr != nil { + return fmt.Errorf("writing %s: %w", path, writeErr) + } + if writeErr = os.Chmod(tmpName, perm); writeErr != nil { + return fmt.Errorf("writing %s: %w", path, writeErr) + } + if writeErr = os.Rename(tmpName, path); writeErr != nil { + return fmt.Errorf("writing %s: %w", path, writeErr) + } + return nil +} + +// addSentinelBlock adds or replaces a sentinel-delimited block in the given +// config file. If the file does not exist, it is created with mode 0644. +// Existing file permissions are preserved. The operation is idempotent: +// calling it twice with the same line produces the same result as calling +// it once. If the file contains orphaned or reversed markers, a new block +// is appended rather than attempting to repair the malformed state. +func addSentinelBlock(configPath, line string) error { + content, perm, err := readConfigFile(configPath) + if err != nil { + return err + } + + newContent := computeAddSentinel(content, line) + return atomicWriteFile(configPath, []byte(newContent), perm) +} + +// removeSentinelBlock removes the sentinel-delimited block from the given +// config file. If the file does not exist, this is a no-op. If the markers +// are orphaned or reversed, the file is left unchanged. Existing file +// permissions are preserved. +func removeSentinelBlock(configPath string) error { + content, perm, err := readConfigFile(configPath) + if err != nil { + return err + } + + // If the file did not exist, readConfigFile returns ("", 0644, nil). + // computeRemoveSentinel("") returns ("", false), so !found handles both + // the missing-file case and the no-block-present case uniformly. + newContent, found := computeRemoveSentinel(content) + if !found { + return nil + } + + return atomicWriteFile(configPath, []byte(newContent), perm) +} + +// manualRemnant represents a line in a shell config file that references the +// completion script but is outside our sentinel-managed block. +type manualRemnant struct { + lineNumber int // 1-based, for display in user-facing warnings + lineText string // trimmed content of the matching line +} + +// findManualRemnants scans a shell config file for lines referencing the +// completion script filename that are outside our sentinel block. This detects +// manually-added source/load lines that the user may need to clean up. +// +// Lines inside the sentinel block, blank lines, and comment lines (starting +// with #) are excluded from the scan. Returns nil if the file cannot be read +// or no matches are found. +func findManualRemnants(configPath, scriptFilename string) []manualRemnant { + data, err := os.ReadFile(configPath) + if err != nil { + return nil + } + + var remnants []manualRemnant + inSentinelBlock := false + + for i, line := range strings.Split(string(data), "\n") { + trimmed := strings.TrimSpace(line) + + if strings.Contains(trimmed, sentinelBegin) { + inSentinelBlock = true + continue + } + if strings.Contains(trimmed, sentinelEnd) { + inSentinelBlock = false + continue + } + + if inSentinelBlock { + continue + } + + // Skip blank lines and comments + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + if strings.Contains(trimmed, scriptFilename) { + remnants = append(remnants, manualRemnant{ + lineNumber: i + 1, + lineText: trimmed, + }) + } + } + + return remnants +} diff --git a/pkg/cmd/sentinel_test.go b/pkg/cmd/sentinel_test.go new file mode 100644 index 00000000..64b5cb55 --- /dev/null +++ b/pkg/cmd/sentinel_test.go @@ -0,0 +1,352 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --------------------------------------------------------------------------- +// Pure function tests — no filesystem needed +// --------------------------------------------------------------------------- + +func TestComputeAddSentinel(t *testing.T) { + tests := []struct { + name string + content string + line string + wantHas []string // substrings that must be present + wantNot []string // substrings that must be absent + wantOnce []string // substrings that must appear exactly once + }{ + { + name: "empty content", + content: "", + line: "source ~/.stripe/stripe-completion.zsh", + wantHas: []string{sentinelBegin, "source ~/.stripe/stripe-completion.zsh", sentinelEnd}, + wantOnce: []string{sentinelBegin, sentinelEnd}, + }, + { + name: "existing content without sentinel", + content: "export PATH=/usr/local/bin:$PATH\n", + line: "source ~/.stripe/stripe-completion.zsh", + wantHas: []string{"export PATH=/usr/local/bin:$PATH\n", sentinelBegin, sentinelEnd}, + wantOnce: []string{sentinelBegin, sentinelEnd}, + }, + { + name: "replace existing block", + content: fmt.Sprintf("before\n%s\nold source line\n%s\nafter\n", sentinelBegin, sentinelEnd), + line: "new source line", + wantHas: []string{"before\n", "new source line", "after\n"}, + wantNot: []string{"old source line"}, + wantOnce: []string{sentinelBegin, sentinelEnd}, + }, + { + name: "orphaned begin only — appends new block", + content: fmt.Sprintf("before\n%s\norphaned source line\nafter\n", sentinelBegin), + line: "new source line", + wantHas: []string{"new source line", sentinelEnd}, + }, + { + name: "orphaned end only — appends new block", + content: fmt.Sprintf("before\n%s\nafter\n", sentinelEnd), + line: "new source line", + wantHas: []string{"new source line", sentinelBegin}, + wantOnce: []string{sentinelBegin}, + }, + { + name: "reversed markers — appends new block", + content: fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin), + line: "new source line", + wantHas: []string{"new source line"}, + }, + { + name: "missing trailing newline — newline inserted before block", + content: "no trailing newline", + line: "source line", + wantHas: []string{"no trailing newline\n" + sentinelBegin}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := computeAddSentinel(tt.content, tt.line) + for _, s := range tt.wantHas { + assert.Contains(t, got, s) + } + for _, s := range tt.wantNot { + assert.NotContains(t, got, s) + } + for _, s := range tt.wantOnce { + assert.Equal(t, 1, strings.Count(got, s), "expected exactly one occurrence of %q", s) + } + }) + } +} + +func TestComputeRemoveSentinel(t *testing.T) { + tests := []struct { + name string + content string + wantFound bool + wantHas []string + wantNot []string + }{ + { + name: "no block present", + content: "export FOO=bar\n", + wantFound: false, + wantHas: []string{"export FOO=bar\n"}, + }, + { + name: "empty content", + content: "", + wantFound: false, + }, + { + name: "valid block removed", + content: fmt.Sprintf("before\n%s\nsource line\n%s\nafter\n", sentinelBegin, sentinelEnd), + wantFound: true, + wantHas: []string{"before\n", "after\n"}, + wantNot: []string{sentinelBegin, sentinelEnd, "source line"}, + }, + { + name: "orphaned begin only — no-op", + content: fmt.Sprintf("before\n%s\norphaned\nafter\n", sentinelBegin), + wantFound: false, + wantHas: []string{sentinelBegin, "orphaned"}, + }, + { + name: "orphaned end only — no-op", + content: fmt.Sprintf("before\n%s\nafter\n", sentinelEnd), + wantFound: false, + wantHas: []string{sentinelEnd}, + }, + { + name: "reversed markers — no-op", + content: fmt.Sprintf("before\n%s\norphaned\n%s\nafter\n", sentinelEnd, sentinelBegin), + wantFound: false, + wantHas: []string{sentinelEnd, sentinelBegin}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, found := computeRemoveSentinel(tt.content) + assert.Equal(t, tt.wantFound, found) + for _, s := range tt.wantHas { + assert.Contains(t, got, s) + } + for _, s := range tt.wantNot { + assert.NotContains(t, got, s) + } + }) + } +} + +// --------------------------------------------------------------------------- +// I/O wrapper tests — addSentinelBlock / removeSentinelBlock +// --------------------------------------------------------------------------- + +func TestAddSentinelBlockNewFile(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + + err := addSentinelBlock(configPath, "source /home/user/.stripe/stripe-completion.zsh") + require.NoError(t, err) + + data, err := os.ReadFile(configPath) + require.NoError(t, err) + + content := string(data) + assert.Contains(t, content, sentinelBegin) + assert.Contains(t, content, "source /home/user/.stripe/stripe-completion.zsh") + assert.Contains(t, content, sentinelEnd) +} + +func TestAddSentinelBlockPreservesPermissions(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0600)) + + err := addSentinelBlock(configPath, "source line") + require.NoError(t, err) + + info, err := os.Stat(configPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") +} + +func TestAddSentinelBlockReadPermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("content"), 0644)) + require.NoError(t, os.Chmod(configPath, 0000)) + t.Cleanup(func() { os.Chmod(configPath, 0644) }) + + err := addSentinelBlock(configPath, "source line") + assert.Error(t, err) +} + +func TestAddSentinelBlockWritePermissionDenied(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + // Atomic write creates a new temp file and renames it. To prevent the write, + // we make the containing directory read-only so CreateTemp fails. + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + require.NoError(t, os.WriteFile(configPath, []byte("existing\n"), 0644)) + require.NoError(t, os.Chmod(dir, 0555)) + t.Cleanup(func() { os.Chmod(dir, 0755) }) + + err := addSentinelBlock(configPath, "source line") + assert.Error(t, err) +} + +func TestRemoveSentinelBlockMissingFile(t *testing.T) { + err := removeSentinelBlock(filepath.Join(t.TempDir(), "nonexistent")) + assert.NoError(t, err) +} + +func TestRemoveSentinelBlockPreservesPermissions(t *testing.T) { + if runtime.GOOS == "windows" || os.Getuid() == 0 { + t.Skip("Cannot test Unix file permissions on Windows or as root") + } + + dir := t.TempDir() + configPath := filepath.Join(dir, ".zshrc") + content := fmt.Sprintf("before\n%s\nline\n%s\nafter\n", sentinelBegin, sentinelEnd) + require.NoError(t, os.WriteFile(configPath, []byte(content), 0600)) + + err := removeSentinelBlock(configPath) + require.NoError(t, err) + + info, err := os.Stat(configPath) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "file permissions should be preserved") +} + +// --------------------------------------------------------------------------- +// findManualRemnants +// --------------------------------------------------------------------------- + +func TestFindManualRemnants(t *testing.T) { + tests := []struct { + name string + content string + scriptFilename string + wantLen int + wantLineNums []int + }{ + { + name: "detects manual source line", + content: "export PATH=/usr/local/bin:$PATH\nsource ~/.stripe/stripe-completion.zsh\nalias ls='ls -G'\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 1, + wantLineNums: []int{2}, + }, + { + name: "detects dot-source syntax", + content: ". /some/custom/path/stripe-completion.bash\n", + scriptFilename: "stripe-completion.bash", + wantLen: 1, + wantLineNums: []int{1}, + }, + { + name: "detects line with other commands", + content: "[ -f ~/.stripe/stripe-completion.zsh ] && source ~/.stripe/stripe-completion.zsh\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 1, + wantLineNums: []int{1}, + }, + { + name: "detects custom path", + content: "source /opt/completions/stripe-completion.zsh\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 1, + }, + { + name: "ignores lines inside sentinel block", + content: fmt.Sprintf("before\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nafter\n", + sentinelBegin, sentinelEnd), + scriptFilename: "stripe-completion.zsh", + wantLen: 0, + }, + { + name: "ignores comment lines", + content: "# source ~/.stripe/stripe-completion.zsh\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 0, + }, + { + name: "no match returns nil", + content: "export PATH=/usr/local/bin:$PATH\nalias ls='ls -G'\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 0, + }, + { + name: "multiple matches", + content: "source ~/.stripe/stripe-completion.zsh\nexport FOO=bar\n. /other/stripe-completion.zsh\n", + scriptFilename: "stripe-completion.zsh", + wantLen: 2, + wantLineNums: []int{1, 3}, + }, + { + name: "manual line before sentinel block only", + content: fmt.Sprintf("source ~/my/stripe-completion.zsh\n%s\nsource ~/.stripe/stripe-completion.zsh\n%s\n", + sentinelBegin, sentinelEnd), + scriptFilename: "stripe-completion.zsh", + wantLen: 1, + wantLineNums: []int{1}, + }, + { + name: "manual line after sentinel block only", + content: fmt.Sprintf("%s\nsource ~/.stripe/stripe-completion.zsh\n%s\nsource ~/custom/stripe-completion.zsh\n", + sentinelBegin, sentinelEnd), + scriptFilename: "stripe-completion.zsh", + wantLen: 1, + wantLineNums: []int{4}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config") + require.NoError(t, os.WriteFile(configPath, []byte(tt.content), 0644)) + + remnants := findManualRemnants(configPath, tt.scriptFilename) + + if tt.wantLen == 0 { + // nil and empty slice are both acceptable for "no results" + assert.Len(t, remnants, 0) + } else { + require.Len(t, remnants, tt.wantLen) + for i, wantLine := range tt.wantLineNums { + assert.Equal(t, wantLine, remnants[i].lineNumber) + } + } + }) + } +} + +func TestFindManualRemnantsReturnsNilForMissingFile(t *testing.T) { + remnants := findManualRemnants(filepath.Join(t.TempDir(), "nonexistent"), "stripe-completion.zsh") + assert.Nil(t, remnants) +} From 79cff4d34710e7893803841d1f78fdf764f366ff Mon Sep 17 00:00:00 2001 From: Mike North Date: Tue, 14 Apr 2026 11:47:53 -0700 Subject: [PATCH 3/3] fix: gofmt alignment in sentinel test table literals Committed-By-Agent: claude --- pkg/cmd/sentinel_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/sentinel_test.go b/pkg/cmd/sentinel_test.go index 64b5cb55..dce8980e 100644 --- a/pkg/cmd/sentinel_test.go +++ b/pkg/cmd/sentinel_test.go @@ -40,11 +40,11 @@ func TestComputeAddSentinel(t *testing.T) { wantOnce: []string{sentinelBegin, sentinelEnd}, }, { - name: "replace existing block", - content: fmt.Sprintf("before\n%s\nold source line\n%s\nafter\n", sentinelBegin, sentinelEnd), - line: "new source line", - wantHas: []string{"before\n", "new source line", "after\n"}, - wantNot: []string{"old source line"}, + name: "replace existing block", + content: fmt.Sprintf("before\n%s\nold source line\n%s\nafter\n", sentinelBegin, sentinelEnd), + line: "new source line", + wantHas: []string{"before\n", "new source line", "after\n"}, + wantNot: []string{"old source line"}, wantOnce: []string{sentinelBegin, sentinelEnd}, }, { @@ -54,9 +54,9 @@ func TestComputeAddSentinel(t *testing.T) { wantHas: []string{"new source line", sentinelEnd}, }, { - name: "orphaned end only — appends new block", - content: fmt.Sprintf("before\n%s\nafter\n", sentinelEnd), - line: "new source line", + name: "orphaned end only — appends new block", + content: fmt.Sprintf("before\n%s\nafter\n", sentinelEnd), + line: "new source line", wantHas: []string{"new source line", sentinelBegin}, wantOnce: []string{sentinelBegin}, },