Skip to content

fix(execution): strip quoted literals and heredoc bodies before path check#20

Merged
genni613 merged 4 commits into
masterfrom
fix/workspace-path-check-literals
Apr 17, 2026
Merged

fix(execution): strip quoted literals and heredoc bodies before path check#20
genni613 merged 4 commits into
masterfrom
fix/workspace-path-check-literals

Conversation

@TuYv

@TuYv TuYv commented Apr 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • hasPathsOutsideWorkspace split the entire command by whitespace, so JavaDoc-style tokens (/**, */) inside quoted args or << 'EOF' heredoc bodies were flagged as absolute paths outside the project and the command was blocked — e.g. cat > Foo.java << 'EOF' ... /** javadoc */ ... EOF, echo '/**', python3 -c "... /** ...".
  • Added a small lexer (stripLiteralsAndHeredocs + consumeHeredoc) that replaces single/double-quoted literals and bash heredoc bodies (quoted, unquoted, <<- tab-stripped) with a single space before tokenization. <<< here-strings are intentionally left to the quote handler.
  • WHITESPACE regex hoisted to file-level, regionMatches used in heredoc line scan to avoid per-line String allocations.

Trade-off: a fully quoted absolute path (cat "/etc/passwd") no longer trips the check. User approval is the real gate — this reduces false positives on legitimate commands, not security.

Test plan

  • ./gradlew test --tests "*ShellPlatformTest*" — 45/45 passing, includes 13 new regression tests (Unix quoted/heredoc variants, Windows quoted drive paths, <<< positive/negative, malformed unclosed quote, heredoc followed by real /etc/passwd).
  • Full ./gradlew test BUILD SUCCESSFUL.
  • Manually verify in a Java project that cat > Foo.java << 'EOF' ... EOF, echo '/** ... */', and python3 -c "... /** ..." commands reach the approval dialog instead of being silently blocked.

…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.
@github-actions

Copy link
Copy Markdown

Failed to generate code suggestions for PR

@TuYv TuYv requested a review from genni613 April 17, 2026 04:59
@genni613

Copy link
Copy Markdown
Collaborator

/review

@github-actions

Copy link
Copy Markdown

Preparing review...

TuYv added 2 commits April 17, 2026 13:46
…g 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.
- 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.
@TuYv

TuYv commented Apr 17, 2026

Copy link
Copy Markdown
Owner Author

/review

@github-actions

Copy link
Copy Markdown

Preparing review...

@genni613 genni613 merged commit afef56f into master Apr 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants