iterative / setup-cml

GitHub Action for CML setup
25 stars 12 forks source link

Build `index.js` on GitHub Actions, **not** as a commit hook #61

Closed 0x2b3bfa0 closed 2 years ago

0x2b3bfa0 commented 2 years ago

Pull requests on this repository include both the changes to the source code and the minified index.js derived from compiling those changes. Nothing keeps me from crafting a pull request with an apparently innocuous source change accompanied by a malicious index.js file.

dacbd commented 2 years ago

maybe something like: https://github.com/dacbd/create-issue-action/blob/a5590b81483f307133066123cfea86635b1d3ee4/.github/workflows/test.yml#L8-L25

  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-node@v3
        with:
          node-version: '16'
      - run: |
          echo "todo"
          npm install
          npm run build
          if [[ -z $(git status --porcelain) ]]; then
            echo "good"
          else
            echo "Forgot to commit 'npm run build'?"
            git status
            exit 1
          fi
0x2b3bfa0 commented 2 years ago

Yes, that would suffice.

0x2b3bfa0 commented 2 years ago

Still, separating the development and release processes might simplify considerably the workflow.

dacbd commented 2 years ago

Still, separating the development and release processes might simplify considerably the workflow.

Sure, security-wise I think that bot commits are not necessarily better. (vs requiring signed commits from humans) I think that having a check that rejects PR's which have "unbuilt changes" is sufficient for this.

0x2b3bfa0 commented 2 years ago

bot commits are not necessarily better [...] vs requiring signed commits from humans

I believe that, in this particular case, bot commits are equivalent to artifacts uploaded during the release process.

Build artifacts aren't usually signed by humans. 🤖

0x2b3bfa0 commented 2 years ago

Anyway, both approaches are equally effective to prevent this issue. 🤷🏼

dacbd commented 2 years ago

I believe that, in this particular case, bot commits are equivalent to artifacts uploaded during the release process.

Build artifacts aren't usually signed by humans. 🤖

Depending on your paranoid level of security for the software supply chain, I think it's easy to play what-if games and argue both sides of this coin.