diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index 12a43a1..cb19798 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -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: @@ -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 }} diff --git a/src/main/kotlin/com/github/codeplangui/execution/ShellPlatform.kt b/src/main/kotlin/com/github/codeplangui/execution/ShellPlatform.kt index 56fa84b..cf7de5d 100644 --- a/src/main/kotlin/com/github/codeplangui/execution/ShellPlatform.kt +++ b/src/main/kotlin/com/github/codeplangui/execution/ShellPlatform.kt @@ -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 @@ -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 @@ -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("\\\\") diff --git a/src/test/kotlin/com/github/codeplangui/execution/ShellPlatformTest.kt b/src/test/kotlin/com/github/codeplangui/execution/ShellPlatformTest.kt index cc27245..6d87bd2 100644 --- a/src/test/kotlin/com/github/codeplangui/execution/ShellPlatformTest.kt +++ b/src/test/kotlin/com/github/codeplangui/execution/ShellPlatformTest.kt @@ -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 @@ -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