From 6243b42a50897d06256862b5d63cdaf0155bb119 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Fri, 17 Apr 2026 12:57:36 +0800 Subject: [PATCH 1/3] fix(execution): strip quoted literals and heredoc bodies before path check The workspace-path safety check tokenized the whole command string by whitespace, so JavaDoc-style tokens such as /** and */ inside `echo` arguments, `cat << 'EOF'` bodies, or `python3 -c` strings were treated as absolute paths outside the project and the command was blocked. Add a small lexer that replaces single/double-quoted literals and bash heredoc bodies (quoted, unquoted, and tab-stripped variants) with a single space before tokenization. User approval remains the primary gate; this only reduces false positives on legitimate commands. --- .../codeplangui/execution/ShellPlatform.kt | 80 ++++++++++++++- .../execution/ShellPlatformTest.kt | 98 +++++++++++++++++++ 2 files changed, 176 insertions(+), 2 deletions(-) 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 From 741142f6e5c958b6a486c3baca3756f37be0b557 Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Fri, 17 Apr 2026 13:44:05 +0800 Subject: [PATCH 2/3] fix(ci): pin MODEL_WEAK and FALLBACK_MODELS for PR-Agent on ARK coding plan The Volcengine ARK coding-plan endpoint only supports a whitelist of models. Setting only CONFIG.MODEL left CONFIG.MODEL_WEAK and CONFIG.FALLBACK_MODELS at PR-Agent's built-in default (o4-mini), which the endpoint rejects with `404 UnsupportedModel`. As a result, pr_description and improve (code suggestions) failed silently every run. Pin all three model slots to MiniMax-M2.5 so every PR-Agent step uses the same coding-plan-compatible model. --- .github/workflows/pr-agent.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index 12a43a1..1adcade 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -34,4 +34,6 @@ 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_TOKEN: ${{ secrets.GITHUB_TOKEN }} From a07245da1b0b5dcfb6f1a5373e7ad1277ead280b Mon Sep 17 00:00:00 2001 From: TuYv <861506831@qq.com> Date: Fri, 17 Apr 2026 13:53:52 +0800 Subject: [PATCH 3/3] ci(pr-agent): auto review on push and gate manual trigger to reviewers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Explicitly enable AUTO_DESCRIBE / AUTO_REVIEW / AUTO_IMPROVE so every push to a PR (`synchronize`) runs describe + review + improve without relying on upstream defaults. - Tighten the job `if`: for issue_comment / pull_request_review_comment events, require `github.event.comment.author_association` to be OWNER / MEMBER / COLLABORATOR. Previously any non-bot commenter on a PR could re-trigger the workflow under pull_request_target, which had access to repository secrets — a fork-PR command-injection hole. - pull_request[_target] events still gate by the PR author's role. Net effect: designated reviewers can manually re-trigger via slash commands (e.g. `/review`), fork authors cannot. --- .github/workflows/pr-agent.yml | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index 1adcade..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: @@ -36,4 +50,7 @@ jobs: 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 }}