Skip to content

Add expression parser for && || and parentheses with tree visitor validation#553

Merged
pelikhan merged 2 commits intomainfrom
copilot/add-expression-parser-support
Sep 8, 2025
Merged

Add expression parser for && || and parentheses with tree visitor validation#553
pelikhan merged 2 commits intomainfrom
copilot/add-expression-parser-support

Conversation

Copy link
Contributor

Copilot AI commented Sep 8, 2025

Problem

The existing expression validation in validateExpressionSafety only handled simple literal expressions within GitHub Actions ${{ }} blocks. It couldn't parse or validate complex expressions using logical operators like &&, ||, !, and parentheses for grouping. This meant that complex expressions like:

${{ (github.event_name == 'issues') && (github.event.action == 'opened') || !github.event.pull_request.draft }}

Would be treated as a single literal and either fully allowed or rejected, rather than validating each individual component expression.

Solution

This PR adds a comprehensive expression parser that extends the existing expressions.go infrastructure:

New Components

  1. ExpressionParser: A recursive descent parser that tokenizes and parses complex expressions with proper operator precedence
  2. ParseExpression(): Main entry point that converts string expressions into the existing ConditionNode AST
  3. VisitExpressionTree(): Tree visitor that traverses expression ASTs and calls a visitor function for each literal expression
  4. Enhanced validation: Updated validateExpressionSafety to use the parser for complex expressions

Supported Syntax

  • Literals: github.event_name == 'issues'
  • AND operations: condition1 && condition2 (higher precedence)
  • OR operations: condition1 || condition2 (lower precedence)
  • NOT operations: !condition1
  • Parentheses: (condition1 && condition2) || condition3
  • Complex nested expressions with proper precedence handling

Implementation Details

The parser follows these design principles:

  • Minimal changes: Builds upon existing ConditionNode infrastructure rather than replacing it
  • Backward compatibility: Falls back to original validation for unparseable expressions
  • Proper precedence: AND (&&) has higher precedence than OR (||)
  • Error handling: Comprehensive error reporting for malformed expressions
  • Security focused: Each literal expression in complex expressions is individually validated

Example Usage

// Parse a complex expression
tree, err := ParseExpression("(github.workflow && github.repository) || github.run_id")
if err != nil {
    return err
}

// Visit each literal expression for validation
err = VisitExpressionTree(tree, func(expr *ExpressionNode) error {
    return validateSingleExpression(expr.Expression, ...)
})

Validation Enhancement

The expression safety validation now:

  1. Attempts to parse complex expressions using the new parser
  2. Visits each literal expression in the parsed tree to validate it against the allowlist
  3. Falls back to the original validation for unparseable expressions
  4. Maintains all existing security guarantees

Testing

Added comprehensive test coverage including:

  • Parser functionality with edge cases and error handling
  • Tree visitor functionality with error propagation
  • Integration tests combining parsing and validation
  • Expression safety validation with complex expressions
  • All existing tests continue to pass

The implementation successfully handles complex expressions while maintaining the security properties of the original validation system.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.contoso.com
    • Triggering command: /tmp/go-build1091059789/b278/cli.test -test.testlogfile=/tmp/go-build1091059789/b278/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)
    • Triggering command: /tmp/go-build1482235784/b278/cli.test -test.testlogfile=/tmp/go-build1482235784/b278/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review September 8, 2025 13:40
Copilot AI changed the title [WIP] Add a simple expression parser that can handle && || , parenthesis. Use parser when validating expressions in agenric workflow markdown by visiting the expression tree and validating literalexpresssions. Extend existing expreessions.go support Add expression parser for && || and parentheses with tree visitor validation Sep 8, 2025
Copilot AI requested a review from pelikhan September 8, 2025 13:41
@pelikhan pelikhan merged commit 5f28796 into main Sep 8, 2025
22 checks passed
@pelikhan pelikhan deleted the copilot/add-expression-parser-support branch September 8, 2025 13:49
Mossaka added a commit that referenced this pull request Feb 6, 2026
Includes fixes for chroot mode MCP server connectivity:
- fix: resolve host.docker.internal DNS in chroot mode (#555)
- fix: inject host.docker.internal from docker-manager.ts
- fix: postprocess smoke workflows to build containers locally
- fix: set NO_PROXY for host gateway to bypass Squid (#554)
- fix: bypass Squid for network gateway to fix MCP SSE crash (#553)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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