Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If condition are always evaluated as true when containing expression syntax inside #1173

Open
2 tasks
dlavrenuek opened this issue Jun 25, 2021 · 37 comments
Open
2 tasks
Labels
bug Something isn't working Runner Bug Bug fix scope to the runner

Comments

@dlavrenuek
Copy link

Describe the bug
An if condition in a step definition that includes expression syntax in condition body is always evaluated as false positive or is ignored.

Example workflow: https://github.com/dlavrenuek/test-workflow-if-condition/blob/master/.github/workflows/test.yml

name: test if conditions

on:
  push:
    branches: [master, test]

env:
  TEST_BRANCH: "test"
  EMPTY_VALUE: ""

jobs:
  test-conditions:
    runs-on: ubuntu-latest

    steps:
      - name: simple evaluation on master
        if: github.ref == 'refs/heads/master'
        run: echo "runs"

      - name: simple evaluation on test
        if: github.ref == 'refs/heads/test'
        run: echo "runs"

      - name: false positive with expression syntac evaluation
        if: github.ref == 'refs/head/${{ env.TEST_BRANCH }}' # this condition is ignored or is evaluated as false positive
        run: echo "runs"

      - name: false positive with expression syntac evaluation
        if: false && '${{ env.EMPTY_VALUE }}' # this condition is ignored or is evaluated as false positive
        run: echo "runs"

Example run: https://github.com/dlavrenuek/test-workflow-if-condition/runs/2914408827?check_suite_focus=true

Example step definition:

      - name: false positive with expression syntax evaluation
        if: github.ref == 'refs/head/${{ env.TEST_BRANCH }}'
        run: echo "runs"

In the example workflow the last two steps should not have been run, but they were

To Reproduce
Steps to reproduce the behavior:

  1. create a workflow
  2. add an if condition with expression syntax ${{ ... }} as part of the condition (not wrapping the whole condition)
  3. trigger the workflow

Expected behavior

  • The if condition should be correctly evaluated.
  • In case that the expression syntax is not supported as part of an if condition - an error should be thrown instead of silently ignoring it.

Runner Version and Platform

GitHub platform

What's not working?

The steps are executed because of false positive evaluation of he if condition containing expression syntax

Bildschirmfoto 2021-06-25 um 15 31 17

@dlavrenuek dlavrenuek added the bug Something isn't working label Jun 25, 2021
@TingluoHuang
Copy link
Member

@dlavrenuek you can enable debug log to see how the expression gets evaluated.
https://docs.github.com/en/actions/managing-workflow-runs/enabling-debug-logging#enabling-step-debug-logging

@dlavrenuek
Copy link
Author

@TingluoHuang Here are the logs for the step:

      - name: false positive with expression syntax evaluation
        if: github.ref == 'refs/head/${{ env.TEST_BRANCH }}' # is always executed
        run: echo "runs"
##[debug]Evaluating condition for step: 'false positive with expression syntax evaluation'
##[debug]Evaluating: (success() && format('github.ref == ''refs/head/{0}''', env.TEST_BRANCH))
##[debug]Evaluating And:
##[debug]..Evaluating success:
##[debug]..=> true
##[debug]..Evaluating format:
##[debug]....Evaluating String:
##[debug]....=> 'github.ref == ''refs/head/{0}'''
##[debug]....Evaluating Index:
##[debug]......Evaluating env:
##[debug]......=> Object
##[debug]......Evaluating String:
##[debug]......=> 'TEST_BRANCH'
##[debug]....=> 'test'
##[debug]..=> 'github.ref == ''refs/head/test'''
##[debug]=> 'github.ref == ''refs/head/test'''
##[debug]Expanded: (true && 'github.ref == ''refs/head/test''')
##[debug]Result: 'github.ref == ''refs/head/test'''
##[debug]Starting: false positive with expression syntax evaluation
...

@dlavrenuek
Copy link
Author

And logs for

      - name: false positive with expression syntax evaluation
        if: false && '${{ env.EMPTY_VALUE }}' # is always executed
        run: echo "runs"
##[debug]Evaluating condition for step: 'false positive with expression syntax evaluation'
##[debug]Evaluating: (success() && format('false && ''{0}''', env.EMPTY_VALUE))
##[debug]Evaluating And:
##[debug]..Evaluating success:
##[debug]..=> true
##[debug]..Evaluating format:
##[debug]....Evaluating String:
##[debug]....=> 'false && ''{0}'''
##[debug]....Evaluating Index:
##[debug]......Evaluating env:
##[debug]......=> Object
##[debug]......Evaluating String:
##[debug]......=> 'EMPTY_VALUE'
##[debug]....=> ''
##[debug]..=> 'false && '''''
##[debug]=> 'false && '''''
##[debug]Expanded: (true && 'false && ''''')
##[debug]Result: 'false && '''''
##[debug]Starting: false positive with expression syntax evaluation

@rgr
Copy link

rgr commented Aug 26, 2021

any solution?

@dvishniakov
Copy link

@TingluoHuang , can you please remove the awaiting-customer-responselabel? Response with debug output was provided.

@dvishniakov
Copy link

I've opened a #1395 with a similar result, but the syntax is if: ( ${{ expression}} ). Seems like logical grouping brackets around expression confuses the engine.

@eb-mikegorecki
Copy link

Bump

@gsingh1
Copy link

gsingh1 commented Oct 23, 2021

I ran into this issue recently too and the way I got expressions to work in if correctly was to use the format function. Please try this:

      - name: false positive with expression syntax evaluation
        if: github.ref == format('refs/heads/{0}', env.TEST_BRANCH)
        run: echo "::error::This step should be skipped and not run!" && false

@fenollp
Copy link

fenollp commented Jan 25, 2022

        if: false && '${{ env.EMPTY_VALUE }}' # is always executed

The string '${{ env.EMPTY_VALUE }}' is truthy so this expression evaluates to true. See my other comment: #1395 (comment)

@prashil-g
Copy link

+1

@GabrieleCalarota
Copy link

+1 How can we solve this?

cgr71ii referenced this issue in bitextor/bitextor Mar 7, 2022
cgr71ii added a commit to bitextor/bitextor that referenced this issue Mar 8, 2022
From full to lite model (Bicleaner AI)

actions/runner#1173
@ThePieMonster
Copy link

Having the same issue with the if statement check as shown below. Always returns true...

- run: |
    if [[ ${{ env.TAGCHECK }} == "true" ]] ; then echo "Hello!" ; else echo "Goodbye!" ; fi
    if [[ "${{ env.TAGCHECK }}" == "true" ]] ; then echo "Hello!" ; else echo "Goodbye!" ; fi
    if [[ ${{ env.TAGCHECK }} == "false" ]] ; then echo "Hello!" ; else echo "Goodbye!" ; fi
    if [[ "${{ env.TAGCHECK }}" == "false" ]] ; then echo "Hello!" ; else echo "Goodbye!" ; fi

- name: Create Tag
  if: ${{ env.TAGCHECK }} == "false"
  uses: actions/github-script@v5
  with:
    script: |
       code...
      })

image

@BuckinghamIO
Copy link

How can this still be an issue almost a year on? Kinda disappointed really as this is quite a fundamental part.

@raghuvarmabh
Copy link

After spending lot of time understanding this i achieved what i want by comparing with the string values of true false from within the run command instead of if:.

@WoodyWoodsta
Copy link

Honestly losing faith in actions - they're just not stable and powerful enough to run organisation-level workflows.

@raghuvarmabh
Copy link

I fixed like this

@joshghent
Copy link

I had the same issue
if: ${{ inputs.environment }} == 'prod'
Screenshot 2022-07-27 at 17 56 29

Really don't get it. I am in the process of moving from Azure DevOps but this is a blocker.

@dlavrenuek
Copy link
Author

@joshghent this should work in your case:

if: inputs.environment == 'prod'

@FeeeeK
Copy link

FeeeeK commented Mar 26, 2023

If you still don't think it needs to be fixed, here is an example of a vulnerability based on the documentation and unexpected behavior due to this bug:
https://github.com/FeeeeK/every-pr-here-will-be-merged

Jacajack added a commit to Jacajack/hdl that referenced this issue Apr 14, 2023
schandrika added a commit to schandrika/test_github_actions that referenced this issue May 3, 2023
schandrika added a commit to schandrika/test_github_actions that referenced this issue May 3, 2023
@kimberleyhallifax
Copy link

I'm encountering the same issue, this always evaluates to true

if: ${{ env.FILE_CHANGED }} != "0"

And this is syntactically incorrect so I can't use it:

if: env.FILE_CHANGED != "0"

@unlikelyzero
Copy link

What year is it?

@thejeff77
Copy link

Just needs to be fixed in the documentation... it says all over to use the ${{ }} syntax which is invalid for if statements.

@Sohaib112
Copy link

Sohaib112 commented Jul 13, 2023

I am having the same issue.
the value of app_found is false in the previous step but it still runs terraform init.

- name: Terraform init
  if: steps.check.outputs.app_found == 'true'
  run: |
    cd ${{ steps.changed-apps.outputs.path }}
    terraform init -upgrade

Here is the debug log:

##[debug]Evaluating condition for step: 'Terraform init'
##[debug]Evaluating: (success() && (steps.check.outputs.app_found == 'true'))
##[debug]Evaluating And:
##[debug]..Evaluating success:
##[debug]..=> true
##[debug]..Evaluating Equal:
##[debug]....Evaluating Index:
##[debug]......Evaluating Index:
##[debug]........Evaluating Index:
##[debug]..........Evaluating steps:
##[debug]..........=> Object
##[debug]..........Evaluating String:
##[debug]..........=> 'check'
##[debug]........=> Object
##[debug]........Evaluating String:
##[debug]........=> 'outputs'
##[debug]......=> Object
##[debug]......Evaluating String:
##[debug]......=> 'app_found'
##[debug]....=> null
##[debug]....Evaluating String:
##[debug]....=> 'true'
##[debug]..=> false
##[debug]=> false
##[debug]Expanded: (true && (null == 'true'))
##[debug]Result: false

I have tried adding ${{ steps.check.outputs.app_found }} == 'true' But still not worked.
Debug logs for this:

##[debug]Evaluating condition for step: 'Terraform init'
##[debug]Evaluating: (success() && format('({0} == ''true'')', steps.check.outputs.app_found))
##[debug]Evaluating And:
##[debug]..Evaluating success:
##[debug]..=> true
##[debug]..Evaluating format:
##[debug]....Evaluating String:
##[debug]....=> '({0} == ''true'')'
##[debug]....Evaluating Index:
##[debug]......Evaluating Index:
##[debug]........Evaluating Index:
##[debug]..........Evaluating steps:
##[debug]..........=> Object
##[debug]..........Evaluating String:
##[debug]..........=> 'check'
##[debug]........=> Object
##[debug]........Evaluating String:
##[debug]........=> 'outputs'
##[debug]......=> Object
##[debug]......Evaluating String:
##[debug]......=> 'app_found'
##[debug]....=> null
##[debug]..=> '( == ''true'')'
##[debug]=> '( == ''true'')'
##[debug]Expanded: (true && '( == ''true'')')
##[debug]Result: '( == ''true'')'

@ViniciusBastosTR
Copy link

This issue will be fixed?

@AivazNurgaliev
Copy link

@benoit-cty Sir, I am forever grateful for your help! I had been sitting 5 hours on this problem, but your reply really helped. Thank you!

sgoudham added a commit to catppuccin/userstyles that referenced this issue Sep 3, 2023
simonsan added a commit to rustic-rs/rustic_server that referenced this issue Sep 12, 2023
CC: actions/runner#1173
Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com>
@tking16
Copy link

tking16 commented Sep 20, 2023

Github actions suuucks

WoodenMaiden added a commit to WoodenMaiden/gama.ppa that referenced this issue Sep 25, 2023
Seriously the bad syntax in the docs since 2021 !! wtf?
actions/runner#1173

Signed-off-by: WoodenMaiden <yann.pomie@laposte.net>
WoodenMaiden added a commit to WoodenMaiden/gama.ppa that referenced this issue Sep 27, 2023
Seriously the bad syntax in the docs since 2021 !! wtf?
actions/runner#1173

Signed-off-by: WoodenMaiden <yann.pomie@laposte.net>
@cceaaal
Copy link

cceaaal commented Oct 31, 2023

I changed the syntax in the condition from e.g.

if: ${{steps.step_id.outputs.output_var}} == 'string'

to

if: ${{steps.step_id.outputs.output_var == 'string'}}

that seems to work

havardholvik added a commit to whereby/github-actions that referenced this issue Jan 16, 2024
Noticed an error when this action was run even though the actor wasnt
matching. According to actions/runner#1173,
you should be really careful when using ${{ }} inside if statements.
strtgbb added a commit to Altinity/clickhouse-regression that referenced this issue Apr 4, 2024
 - fix setting all suites to aws
 - simplify logic to avoid actions/runner#1173
@raksbi
Copy link

raksbi commented Apr 27, 2024

Wow, it is kinda amazing that, this basic thing does not work as expected.

@FreyGeospatial
Copy link

I changed the syntax in the condition from e.g.

if: ${{steps.step_id.outputs.output_var}} == 'string'

to

if: ${{steps.step_id.outputs.output_var == 'string'}}

that seems to work

Confirming this works. Also found some documentation that may be helpful, regarding the ${{ ... }} syntax for if evaluations:
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Runner Bug Bug fix scope to the runner
Projects
None yet
Development

No branches or pull requests