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

Change Debug to public and add new script injection input #329

Closed
wants to merge 3 commits into from

Conversation

hugo-syn
Copy link
Contributor

@hugo-syn hugo-syn commented Aug 2, 2023

I've changed the Debug method of RuleBase since we can now add new rules.

And I've also added a new entry in BuiltinUntrustedInputs because I already saw something like this where It's possible to inject code in the runner from an opened issue:

name: Test

on:
  issues:
    types: [opened]

jobs:
  dummyjob:
    runs-on: ubuntu-latest
    steps:
      - name: parse issue Body
        id: prepareBody
        shell: pwsh
        run: |
          # this script parse issue and set env variables
          .\scripts\issue-body-parser.ps1 $env:BODY
        env:
          BODY: ${{ github.event.issue.body }}
      
      - name: Injection step
        shell: pwsh
        run: |
          
          # InjectionEntry comes from issue-body-parser.ps1 
          # which parse the body and do something like this
          # Write-Output "InjectionEntry=$dataparsedFromIssue" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append

          .\scripts\my-script.ps1 -param ${{ env.InjectionEntry}}
        
./main env-inject.yml       
env-inject.yml :22:265: "env.*" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions for more details [expression]

@hugo-syn
Copy link
Contributor Author

hugo-syn commented Aug 2, 2023

Some tests failed, but I don't understand the output of the CI / Unit tests workflow

@rhysd
Copy link
Owner

rhysd commented Aug 3, 2023

Doesn't the unit test fail on your machine? Please ensure go test ./... passes before making a PR on local.

@rhysd
Copy link
Owner

rhysd commented Aug 3, 2023

Can you separate this PR into two PRs?

  1. making Debug public
  2. add untrusted value filter pattern

I think 1. can be easily merged but 2. would take time to fix unit tests.

@hugo-syn
Copy link
Contributor Author

hugo-syn commented Aug 3, 2023

Ok sorry, I close this one and I'll open 2 PR thanks !

@hugo-syn hugo-syn closed this Aug 3, 2023
@hugo-syn hugo-syn deleted the dev branch August 3, 2023 16:10
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