Replace pull_request_target workflow with two workflows that upload/download an artifact. #251

dblock · 2024-04-18T15:48:44Z

Description

When we use pull_request_target we check out code that has potentially been modified by the requester, which isn't safe. This PR replaces the workflow with something inspired by https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ - a gather step and a comment step that use an artifact to pass information between them.

Here's a a gather workflow that produces the artifact: https://github.com/dblock/opensearch-api-specification/actions/runs/8740901986
Here's a comment workflow that uses it: https://github.com/dblock/opensearch-api-specification/actions/runs/8740884453

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: dblock <dblock@amazon.com>

github-actions · 2024-04-18T15:50:02Z

API specs implemented for 247/649 (38%) APIs.

.github/workflows/coverage-gather.yml

Xtansia · 2024-04-18T21:57:54Z

.github/workflows/coverage-comment.yml

+      - name: Download Coverage Report
+        uses: actions/github-script@v3.1.0
+        with:
+          script: |
+            var artifacts = await github.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: ${{github.event.workflow_run.id }},
+            });
+            var matchArtifact = artifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "coverage"
+            })[0];
+            var download = await github.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            var fs = require('fs');
+            fs.writeFileSync('${{github.workspace}}/coverage.zip', Buffer.from(download.data));
+      - run: |
+            unzip coverage.zip
+            cat coverage.json


actions/download-artifact@v4 now supports downloading artifacts from other workflow runs: https://github.com/actions/download-artifact?tab=readme-ov-file#download-artifacts-from-other-workflow-runs-or-repositories

Updated. This is much nicer.

Successful comment run: https://github.com/dblock/opensearch-api-specification/actions/runs/8752961841
Comment on: dblock#5

github-actions · 2024-04-19T11:25:10Z

API specs implemented for 247/649 (38%) APIs.

Signed-off-by: dblock <dblock@amazon.com>

github-actions · 2024-04-19T11:43:00Z

API specs implemented for 247/649 (38%) APIs.

dblock · 2024-04-19T12:14:32Z

Also https://code.dblock.org/2024/04/19/commenting-on-pull-requests-in-github-actions.html.

nhtruong · 2024-04-20T01:56:17Z

.github/workflows/coverage-comment.yml

+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          script: |
+            const fs = require('fs');
+            var data = JSON.parse(fs.readFileSync('./coverage.json'));


nit: avoid declaring a variable with var, as you MIGHT overwrite var data declared else where. Use const in this case instead.

additional paths Fixed spec (opensearch-project#246) Signed-off-by: saimedhi <saimedhi@amazon.com> Fixed search_pipeline spec (opensearch-project#253) Signed-off-by: saimedhi <saimedhi@amazon.com> Updated API name: search_pipeline.create to search_pipeline.put (opensearch-project#254) Signed-off-by: saimedhi <saimedhi@amazon.com> Fixed search_pipeline.get spec (opensearch-project#255) Signed-off-by: saimedhi <saimedhi@amazon.com> Filled in Missing Defaults (opensearch-project#249) * Filled in Missing Defaults Signed-off-by: Theo Truong <theotr@amazon.com> * # Wordings Signed-off-by: Theo Truong <theotr@amazon.com> --------- Signed-off-by: Theo Truong <theotr@amazon.com> Replace pull_request_target workflow with two workflows that upload/download an artifact. (opensearch-project#251) * Fix: replace pull_request_target with a download/upload artifact. Signed-off-by: dblock <dblock@amazon.com> * Use upload/download-artifact@v4. Signed-off-by: dblock <dblock@amazon.com> --------- Signed-off-by: dblock <dblock@amazon.com> Fix: var -> const. (opensearch-project#258) Signed-off-by: dblock <dblock@amazon.com> Adds tools linter. (opensearch-project#260) Signed-off-by: dblock <dblock@amazon.com> Fix: '@typescript-eslint/indent': 'warn'. (opensearch-project#262) Signed-off-by: dblock <dblock@amazon.com> Removed default values from param description (opensearch-project#264) Signed-off-by: saimedhi <saimedhi@amazon.com> Generate _opendistro endpoints through merger tool (opensearch-project#257) * Generate _opendistro endpoints through merger tool Signed-off-by: Theo Truong <theotr@amazon.com> * # Renamed `replaced` with `superseded` Signed-off-by: Theo Truong <theotr@amazon.com> * # Rebased DEVELOPER_GUIDE.md Signed-off-by: Theo Truong <theotr@amazon.com> * # Set Tabsize from 4 to 2 Signed-off-by: Theo Truong <theotr@amazon.com> --------- Signed-off-by: Theo Truong <theotr@amazon.com> Add _plugins/_notifications/channels API spec (opensearch-project#256) * Add _plugins/_notifications/channels API Signed-off-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch> Fix obvious lints. (opensearch-project#265) * Fix cosmetic autoformat lints. * Fixed @typescript-eslint/no-unused-vars. * Fixed eqeqeq. * Fixed @typescript-eslint/consistent-type-imports. * Fixed no-useless-return. * Fixed @typescript-eslint/array-type. * Rebased with changes on main. Signed-off-by: dblock <dblock@amazon.com> Update list notification channels url for externalDocs (opensearch-project#267) Signed-off-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch> Co-authored-by: Sokratis Papadopoulos <sokratis.papadopoulos@cern.ch> Updated/Corrected Docs (opensearch-project#270) * Updated/Corrected Docs - README.md - CLIENT_GENERATOR_GUIDE.md - DEVELOPER_GUIDE.md - ./tools/README.md Signed-off-by: Theo Truong <theotr@amazon.com> * # minor corrections Signed-off-by: Theo Truong <theotr@amazon.com> * # minor corrections Signed-off-by: Theo Truong <theotr@amazon.com> --------- Signed-off-by: Theo Truong <theotr@amazon.com> Add lychee github action for links checking (opensearch-project#269) Corrected content type for bulk operations (opensearch-project#275) * Corrected content type for bulk operations Signed-off-by: Theo Truong <theotr@amazon.com> * # linting Signed-off-by: Theo Truong <theotr@amazon.com> --------- Signed-off-by: Theo Truong <theotr@amazon.com> Validate _superseded_operations.yaml against its JSON schema (opensearch-project#276) * Validate _superseded_operations.yaml against its JSON schema Signed-off-by: Theo Truong <theotr@amazon.com> * # lint Signed-off-by: Theo Truong <theotr@amazon.com> * # lint Signed-off-by: Theo Truong <theotr@amazon.com> --------- Signed-off-by: Theo Truong <theotr@amazon.com> Fixed Linting for Tools (opensearch-project#278) Signed-off-by: Theo Truong <theotr@amazon.com> Removed Root file since it's not needed anymore (opensearch-project#279) Signed-off-by: Theo Truong <theotr@amazon.com> Fixed missing global params (opensearch-project#280) Signed-off-by: Theo Truong <theotr@amazon.com> Added validation for _info.yaml (opensearch-project#281) DRY'ed JSON schema validation logic Signed-off-by: Theo Truong <theotr@amazon.com> Implement inline object schema validator (opensearch-project#282) * Implement inline object schema validator and underlying visitor pattern Signed-off-by: Thomas Farr <tsfarr@amazon.com> * Fix spec lint error Signed-off-by: Thomas Farr <tsfarr@amazon.com> --------- Signed-off-by: Thomas Farr <tsfarr@amazon.com> oops fix lint

Fix: replace pull_request_target with a download/upload artifact.

e06ce32

Signed-off-by: dblock <dblock@amazon.com>

dblock requested review from harshavamsi, pgtgrly, sachetalva, nhtruong, Xtansia and VachaShah as code owners April 18, 2024 15:48

dblock changed the title ~~Fix: replace pull_request_target with a download/upload artifact.~~ Replace pull_request_target workflow with two workflows that upload/download an artifact. Apr 18, 2024

Xtansia requested changes Apr 18, 2024

View reviewed changes

Use upload/download-artifact@v4.

f0a50bf

Signed-off-by: dblock <dblock@amazon.com>

dblock force-pushed the use-hunter-gather-workflow branch from 7ef04a1 to f0a50bf Compare April 19, 2024 11:41

dblock requested a review from Xtansia April 19, 2024 11:43

nhtruong reviewed Apr 20, 2024

View reviewed changes

Xtansia approved these changes Apr 21, 2024

View reviewed changes

dblock merged commit 61f8a38 into opensearch-project:main Apr 22, 2024
4 checks passed

dblock deleted the use-hunter-gather-workflow branch April 22, 2024 10:47

dblock mentioned this pull request Apr 22, 2024

Fix: var -> const. #258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace pull_request_target workflow with two workflows that upload/download an artifact. #251

Replace pull_request_target workflow with two workflows that upload/download an artifact. #251

dblock commented Apr 18, 2024

github-actions bot commented Apr 18, 2024

Xtansia Apr 18, 2024

dblock Apr 19, 2024

github-actions bot commented Apr 19, 2024

github-actions bot commented Apr 19, 2024

dblock commented Apr 19, 2024

nhtruong Apr 20, 2024 •

edited

Loading

Replace pull_request_target workflow with two workflows that upload/download an artifact. #251

Replace pull_request_target workflow with two workflows that upload/download an artifact. #251

Conversation

dblock commented Apr 18, 2024

Description

github-actions bot commented Apr 18, 2024

Xtansia Apr 18, 2024

Choose a reason for hiding this comment

dblock Apr 19, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 19, 2024

github-actions bot commented Apr 19, 2024

dblock commented Apr 19, 2024

nhtruong Apr 20, 2024 • edited Loading

Choose a reason for hiding this comment

nhtruong Apr 20, 2024 •

edited

Loading