Skip to content
Merged
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
33 changes: 26 additions & 7 deletions .github/workflows/pr-agent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,29 @@ permissions:

jobs:
pr_agent:
# Only run for trusted authors. External contributors' fork PRs will be
# skipped to avoid leaking secrets via workflow injection.
# Gate by actor identity to avoid leaking secrets on fork PRs:
# - pull_request[_target] events: the PR author must be a trusted role
# (OWNER / MEMBER / COLLABORATOR) → auto-runs describe + review on
# every push (`synchronize`).
# - issue_comment / pull_request_review_comment: the commenter must be
# a trusted role → lets designated reviewers manually re-trigger via
# PR-Agent slash commands (e.g. `/review`, `/improve`).
if: >
github.event.sender.type != 'Bot' &&
(github.event.pull_request.author_association == 'OWNER' ||
github.event.pull_request.author_association == 'MEMBER' ||
github.event.pull_request.author_association == 'COLLABORATOR' ||
github.event_name == 'issue_comment' ||
github.event_name == 'pull_request_review_comment')
(
(
(github.event_name == 'pull_request' || github.event_name == 'pull_request_target') &&
(github.event.pull_request.author_association == 'OWNER' ||
github.event.pull_request.author_association == 'MEMBER' ||
github.event.pull_request.author_association == 'COLLABORATOR')
) ||
(
(github.event_name == 'issue_comment' || github.event_name == 'pull_request_review_comment') &&
(github.event.comment.author_association == 'OWNER' ||
github.event.comment.author_association == 'MEMBER' ||
github.event.comment.author_association == 'COLLABORATOR')
)
)
runs-on: ubuntu-latest
name: Run PR-Agent
steps:
Expand All @@ -34,4 +48,9 @@ jobs:
OPENAI_API_BASE: https://ark.cn-beijing.volces.com/api/coding/v3
CONFIG__AI_PROVIDER: openai
CONFIG__MODEL: MiniMax-M2.5
CONFIG__MODEL_WEAK: MiniMax-M2.5
CONFIG__FALLBACK_MODELS: MiniMax-M2.5
GITHUB_ACTION_CONFIG__AUTO_DESCRIBE: "true"
GITHUB_ACTION_CONFIG__AUTO_REVIEW: "true"
GITHUB_ACTION_CONFIG__AUTO_IMPROVE: "true"
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
80 changes: 78 additions & 2 deletions src/main/kotlin/com/github/codeplangui/execution/ShellPlatform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,80 @@ import kotlinx.serialization.json.buildJsonObject
import kotlinx.serialization.json.put
import java.io.File

private val WHITESPACE = Regex("\\s+")

private fun Char.isSpaceOrTab() = this == ' ' || this == '\t'

// Trade-off: a fully quoted absolute path such as `cat "/etc/passwd"` no
// longer trips the workspace check. User approval remains the real gate —
// this helper reduces false positives on legitimate commands (e.g. Java
// source written via heredoc), not security.
private fun stripLiteralsAndHeredocs(command: String): String {
val n = command.length
val out = StringBuilder(n)
var i = 0
while (i < n) {
val c = command[i]
when {
c == '\'' || c == '"' -> {
val end = command.indexOf(c, i + 1)
out.append(' ')
i = if (end < 0) n else end + 1
}
c == '<' && i + 1 < n && command[i + 1] == '<' && (i + 2 >= n || command[i + 2] != '<') -> {
val end = consumeHeredoc(command, i)
if (end > i) { out.append(' '); i = end } else { out.append(c); i++ }
}
else -> { out.append(c); i++ }
}
}
return out.toString()
}

// Returns the index just past the heredoc closing delimiter, or `start` if
// the construct is not a well-formed heredoc. The closing delimiter is
// matched even with leading whitespace (pragmatic relaxation of bash's
// tab-only `<<-` rule).
private fun consumeHeredoc(command: String, start: Int): Int {
val n = command.length
var j = start + 2
if (j < n && command[j] == '-') j++
while (j < n && command[j].isSpaceOrTab()) j++

val quoteChar = if (j < n && (command[j] == '\'' || command[j] == '"')) command[j] else null
if (quoteChar != null) j++
val delimStart = j
val delimEnd = if (quoteChar != null) {
val close = command.indexOf(quoteChar, j)
if (close < 0) return start
close
} else {
var k = j
while (k < n && (command[k].isLetterOrDigit() || command[k] == '_')) k++
k
}
val delimLen = delimEnd - delimStart
if (delimLen == 0) return start
j = delimEnd + if (quoteChar != null) 1 else 0

var pos = j
while (pos < n) {
val nl = command.indexOf('\n', pos)
if (nl < 0) return n
var ls = nl + 1
while (ls < n && command[ls].isSpaceOrTab()) ls++
if (ls + delimLen <= n &&
command.regionMatches(ls, command, delimStart, delimLen)) {
val after = ls + delimLen
if (after == n || command[after] == '\n' || command[after].isSpaceOrTab()) {
return if (after < n && command[after] == '\n') after + 1 else after
}
}
pos = nl + 1
}
return n
}

sealed class ShellPlatform {

abstract fun buildProcess(command: String, workDir: File): ProcessBuilder
Expand Down Expand Up @@ -36,7 +110,8 @@ sealed class ShellPlatform {
override fun hasPathsOutsideWorkspace(command: String, basePath: String): Boolean {
val home = System.getProperty("user.home") ?: ""
val normalizedBase = basePath.trimEnd('/')
return command.split("\\s+".toRegex()).any { token ->
val stripped = stripLiteralsAndHeredocs(command)
return stripped.split(WHITESPACE).any { token ->
if (token.startsWith('-')) return@any false
val expanded = if (token.startsWith("~/")) home + token.drop(1) else token
if (!expanded.startsWith('/')) return@any false
Expand Down Expand Up @@ -106,7 +181,8 @@ sealed class ShellPlatform {

override fun hasPathsOutsideWorkspace(command: String, basePath: String): Boolean {
val normalizedBase = basePath.replace('/', '\\').trimEnd('\\')
return command.split("\\s+".toRegex()).any { token ->
val stripped = stripLiteralsAndHeredocs(command)
return stripped.split(WHITESPACE).any { token ->
if (token.startsWith('-')) return@any false
val normalized = token.replace('/', '\\')
val isAbsolute = normalized.matches(Regex("[A-Za-z]:\\\\.*")) || normalized.startsWith("\\\\")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,80 @@ class ShellPlatformTest {
))
}

// ── Unix.hasPathsOutsideWorkspace: quoted / heredoc stripping ────

@Test
fun `Unix hasPathsOutside ignores javadoc inside single quotes`() {
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"echo '/** comment */' > ./src/Foo.java", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside ignores javadoc inside double quotes`() {
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"echo \"/** comment */\" > ./src/Foo.java", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside ignores heredoc body with quoted delimiter`() {
val cmd = "cat > ./src/Foo.java << 'EOF'\n/**\n * javadoc\n */\npublic class Foo {}\nEOF\n"
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(cmd, "/home/user/project"))
}

@Test
fun `Unix hasPathsOutside ignores heredoc body with unquoted delimiter`() {
val cmd = "cat > ./src/Foo.java << EOF\n/etc/passwd is just text here\nEOF\n"
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(cmd, "/home/user/project"))
}

@Test
fun `Unix hasPathsOutside ignores tab-stripped heredoc body`() {
val cmd = "cat <<-EOF\n\t/** stuff */\n\tEOF\n"
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(cmd, "/home/user/project"))
}

@Test
fun `Unix hasPathsOutside still catches absolute path after heredoc`() {
val cmd = "cat << 'EOF'\n/** safe body */\nEOF\ncat /etc/passwd"
assertTrue(ShellPlatform.Unix.hasPathsOutsideWorkspace(cmd, "/home/user/project"))
}

@Test
fun `Unix hasPathsOutside still catches absolute path between quoted segments`() {
assertTrue(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"echo 'hello' /etc/passwd \"world\"", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside handles malformed unclosed quote without crashing`() {
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"echo 'dangling /** quote", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside treats here-string as regular quoted arg`() {
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"grep foo <<< '/** not a path */'", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside does not treat here-string body as heredoc`() {
assertTrue(ShellPlatform.Unix.hasPathsOutsideWorkspace(
"grep foo <<< /etc/passwd", "/home/user/project"
))
}

@Test
fun `Unix hasPathsOutside handles python inline script with javadoc string`() {
val cmd = "python3 -c \"open('./src/Foo.java','w').write('/** javadoc */')\""
assertFalse(ShellPlatform.Unix.hasPathsOutsideWorkspace(cmd, "/home/user/project"))
}

// ── Windows.hasPathsOutsideWorkspace ─────────────────────────────

@Test
Expand Down Expand Up @@ -160,6 +234,30 @@ class ShellPlatformTest {
))
}

@Test
fun `Windows hasPathsOutside ignores drive path inside single quotes`() {
assertFalse(ShellPlatform.Windows.hasPathsOutsideWorkspace(
"Write-Output 'C:\\Windows\\System32 is just text'",
"C:\\Users\\user\\project"
))
}

@Test
fun `Windows hasPathsOutside ignores drive path inside double quotes`() {
assertFalse(ShellPlatform.Windows.hasPathsOutsideWorkspace(
"Write-Output \"C:\\Windows\\System32 is just text\"",
"C:\\Users\\user\\project"
))
}

@Test
fun `Windows hasPathsOutside still catches drive path between quoted segments`() {
assertTrue(ShellPlatform.Windows.hasPathsOutsideWorkspace(
"Write-Output 'safe' C:\\Windows\\System32\\hosts \"also safe\"",
"C:\\Users\\user\\project"
))
}

// ── toolName ─────────────────────────────────────────────────────

@Test
Expand Down
Loading