Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,4 +196,136 @@ describe("toolExecuteBefore", () => {
expect(mockOutput.args.command).toBe('snip echo "hello | world" | cat')
})
})

describe("PowerShell support", () => {
it("should skip PowerShell env var assignment", async () => {
mockOutput.args.command = "$env:CI='true'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("$env:CI='true'")
})

it("should skip PowerShell variable assignment", async () => {
mockOutput.args.command = "$x = 1"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("$x = 1")
})

it("should skip Write-Output cmdlet", async () => {
mockOutput.args.command = "Write-Output 'hello'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("Write-Output 'hello'")
})

it("should skip Get-ChildItem cmdlet", async () => {
mockOutput.args.command = "Get-ChildItem ."
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("Get-ChildItem .")
})

it("should skip Remove-Item cmdlet", async () => {
mockOutput.args.command = "Remove-Item -Recurse -Force dir"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("Remove-Item -Recurse -Force dir")
})

it("should skip ForEach-Object cmdlet (camelCase verb)", async () => {
mockOutput.args.command = "ForEach-Object { $_.Name }"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("ForEach-Object { $_.Name }")
})

it("should skip ConvertTo-Json cmdlet (camelCase verb)", async () => {
mockOutput.args.command = "ConvertTo-Json -Depth 5"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("ConvertTo-Json -Depth 5")
})

it("should skip PowerShell call operator (&)", async () => {
mockOutput.args.command = "& 'C:\\Program Files\\tool.exe'"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("& 'C:\\Program Files\\tool.exe'")
})

it("should skip PowerShell splatting (@args)", async () => {
mockOutput.args.command = "@args"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("@args")
})

it("should skip PowerShell array literal (@())", async () => {
mockOutput.args.command = '@("a","b")'
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe('@("a","b")')
})

it("should skip env var but snip chained command", async () => {
mockOutput.args.command = "$env:CI='true'; git log -1"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("$env:CI='true'; snip git log -1")
})

it("should skip cmdlet but snip chained command", async () => {
mockOutput.args.command = "Write-Output 'test'; git log -1"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("Write-Output 'test'; snip git log -1")
})

it("should handle mixed PowerShell env vars and commands", async () => {
mockOutput.args.command = "$env:CI='true'; $env:GIT_PAGER='cat'; cd 'C:\\Projects'; git log -1"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("$env:CI='true'; $env:GIT_PAGER='cat'; cd 'C:\\Projects'; snip git log -1")
})
})

describe("newline splitting", () => {
it("should split and snip commands separated by newlines", async () => {
mockOutput.args.command = "git log\ngit status"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip git log\nsnip git status")
})

it("should handle newline after unproxyable command", async () => {
mockOutput.args.command = "cd /tmp\ngit log"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp\nsnip git log")
})

it("should handle mixed newlines and operators", async () => {
mockOutput.args.command = "cd /tmp\ngit log && git status"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp\nsnip git log && snip git status")
})

it("should handle pipe within newline-separated commands", async () => {
mockOutput.args.command = "cd /tmp\ngit log | head"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("cd /tmp\nsnip git log | head")
})

it("should handle PowerShell prelude with newlines", async () => {
mockOutput.args.command = "$env:CI='true'; cd 'C:\\Projects'\ngit show abc123"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("$env:CI='true'; cd 'C:\\Projects'\nsnip git show abc123")
})
})

describe("heredoc safety", () => {
it("should not split heredoc body on newlines", async () => {
mockOutput.args.command = "cat <<EOF\nhello world\nEOF"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat <<EOF\nhello world\nEOF")
})

it("should not split heredoc with quoted delimiter", async () => {
mockOutput.args.command = "cat <<'EOF'\nhello world\nEOF"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat <<'EOF'\nhello world\nEOF")
})

it("should still split operators when heredoc is present", async () => {
mockOutput.args.command = "cat <<EOF\ndata\nEOF && echo done"
await toolExecuteBefore(mockInput, mockOutput)
expect(mockOutput.args.command).toBe("snip cat <<EOF\ndata\nEOF && snip echo done")
})
})
})
49 changes: 36 additions & 13 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,24 @@ const ENV_VAR_RE = /^([A-Za-z_][A-Za-z0-9_]*=[^\s]* +)*/
const UNPROXYABLE_COMMANDS = new Set([
"cd", "source", ".", "export", "alias", "unset", "set", "shopt", "eval", "exec",
])
const OPERATOR_RE = /(\s*(?:&&|\|\||;)\s*|\s&\s?)/
const OPERATOR_RE = /(\s*(?:&&|\|\||;)\s*|\s&\s?|\r?\n)/
Comment thread
greptile-apps[bot] marked this conversation as resolved.
// Same as OPERATOR_RE but without newline splitting. Used when the command
// contains a heredoc (<<DELIM) whose multi-line body must not be split.
const OPERATOR_ONLY_RE = /(\s*(?:&&|\|\||;)\s*|\s&\s?)/

// Heredoc marker: <<DELIM, <<-DELIM, <<'DELIM', <<"DELIM"
const HEREDOC_RE = /<<-?\s*['"]?\w/

// PowerShell: segments starting with these chars are never external commands.
// $ → variable/assignment ($env:CI='true', $x = 1)
// @ → here-string, splat, array (@", @(), @{})
// & → call operator (& "path\to\exe")
// { → script block
const POWERSHELL_SKIP_RE = /^[$@&{]/
Comment on lines +15 to +20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 @-prefix path lacks test coverage

POWERSHELL_SKIP_RE includes @ to handle PowerShell here-strings (@"), splatting (@args), and array literals (@()), but there is no corresponding test. A test for e.g. @args or @("a","b") would confirm the skip path actually works and guard against future refactors.

Suggested change
// PowerShell: segments starting with these chars are never external commands.
// $ → variable/assignment ($env:CI='true', $x = 1)
// @ → here-string, splat, array (@", @(), @{})
// & → call operator (& "path\to\exe")
// { → script block
const POWERSHELL_SKIP_RE = /^[$@&{]/
// PowerShell: segments starting with these chars are never external commands.
// $ → variable/assignment ($env:CI='true', $x = 1)
// @ → here-string, splat, array (@", @(), @{})
// & → call operator (& "path\to\exe")
// { → script block
// NOTE: add a test for @-prefixed segments (splatting / here-strings) in index.test.ts
const POWERSHELL_SKIP_RE = /^[$@&{]/

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


// PowerShell Verb-Noun cmdlets (e.g. Write-Output, Get-ChildItem, Set-Location).
// These are shell builtins and cannot be exec'd by snip.
const POWERSHELL_CMDLET_RE = /^[A-Z][a-zA-Z]*-[A-Z]/

function findFirstPipe(command: string): number {
let inSingleQuote = false
Expand Down Expand Up @@ -33,34 +50,40 @@ function snipCommand(command: string): string {
const envPrefix = (command.match(ENV_VAR_RE) ?? [""])[0]
const bareCmd = command.slice(envPrefix.length).trim()
if (!bareCmd) return command
if (UNPROXYABLE_COMMANDS.has(bareCmd.split(/\s+/)[0])) return command
const firstWord = bareCmd.split(/\s+/)[0]
if (UNPROXYABLE_COMMANDS.has(firstWord)) return command
if (POWERSHELL_SKIP_RE.test(bareCmd)) return command
if (POWERSHELL_CMDLET_RE.test(firstWord)) return command
return `${envPrefix}snip ${bareCmd}`
}

function snipSegment(segment: string): string {
const pipeIdx = findFirstPipe(segment)
if (pipeIdx !== -1) {
const firstCmd = segment.slice(0, pipeIdx).trimEnd()
const rest = segment.slice(pipeIdx)
return snipCommand(firstCmd) + ' ' + rest
}
return snipCommand(segment)
}

export const toolExecuteBefore: NonNullable<Hooks["tool.execute.before"]> = async (input, output) => {
if (input.tool !== "bash") return

const command = output.args.command
if (!command || typeof command !== "string") return
if (command.startsWith("snip ")) return

if (findFirstPipe(command) !== -1) {
const pipeIdx = findFirstPipe(command)
const firstCmd = command.slice(0, pipeIdx).trimEnd()
const rest = command.slice(pipeIdx)
output.args.command = snipCommand(firstCmd) + ' ' + rest
return
}

const segments = command.split(OPERATOR_RE)
const separator = HEREDOC_RE.test(command) ? OPERATOR_ONLY_RE : OPERATOR_RE
const segments = command.split(separator)

if (segments.length === 1) {
output.args.command = snipCommand(command)
output.args.command = snipSegment(command)
return
}

output.args.command = segments
.map((segment) => OPERATOR_RE.test(segment) ? segment : snipCommand(segment))
.map((segment) => separator.test(segment) ? segment : snipSegment(segment))
.join("")
}

Expand Down