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 <[email protected]>

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/[email protected]
+        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 <[email protected]>

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 <[email protected]> Fixed search_pipeline spec (opensearch-project#253) Signed-off-by: saimedhi <[email protected]> Updated API name: search_pipeline.create to search_pipeline.put (opensearch-project#254) Signed-off-by: saimedhi <[email protected]> Fixed search_pipeline.get spec (opensearch-project#255) Signed-off-by: saimedhi <[email protected]> Filled in Missing Defaults (opensearch-project#249) * Filled in Missing Defaults Signed-off-by: Theo Truong <[email protected]> * # Wordings Signed-off-by: Theo Truong <[email protected]> --------- Signed-off-by: Theo Truong <[email protected]> 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 <[email protected]> * Use upload/download-artifact@v4. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]> Fix: var -> const. (opensearch-project#258) Signed-off-by: dblock <[email protected]> Adds tools linter. (opensearch-project#260) Signed-off-by: dblock <[email protected]> Fix: '@typescript-eslint/indent': 'warn'. (opensearch-project#262) Signed-off-by: dblock <[email protected]> Removed default values from param description (opensearch-project#264) Signed-off-by: saimedhi <[email protected]> Generate _opendistro endpoints through merger tool (opensearch-project#257) * Generate _opendistro endpoints through merger tool Signed-off-by: Theo Truong <[email protected]> * # Renamed `replaced` with `superseded` Signed-off-by: Theo Truong <[email protected]> * # Rebased DEVELOPER_GUIDE.md Signed-off-by: Theo Truong <[email protected]> * # Set Tabsize from 4 to 2 Signed-off-by: Theo Truong <[email protected]> --------- Signed-off-by: Theo Truong <[email protected]> Add _plugins/_notifications/channels API spec (opensearch-project#256) * Add _plugins/_notifications/channels API Signed-off-by: Sokratis Papadopoulos <[email protected]> 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 <[email protected]> Update list notification channels url for externalDocs (opensearch-project#267) Signed-off-by: Sokratis Papadopoulos <[email protected]> Co-authored-by: Sokratis Papadopoulos <[email protected]> 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 <[email protected]> * # minor corrections Signed-off-by: Theo Truong <[email protected]> * # minor corrections Signed-off-by: Theo Truong <[email protected]> --------- Signed-off-by: Theo Truong <[email protected]> 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 <[email protected]> * # linting Signed-off-by: Theo Truong <[email protected]> --------- Signed-off-by: Theo Truong <[email protected]> Validate _superseded_operations.yaml against its JSON schema (opensearch-project#276) * Validate _superseded_operations.yaml against its JSON schema Signed-off-by: Theo Truong <[email protected]> * # lint Signed-off-by: Theo Truong <[email protected]> * # lint Signed-off-by: Theo Truong <[email protected]> --------- Signed-off-by: Theo Truong <[email protected]> Fixed Linting for Tools (opensearch-project#278) Signed-off-by: Theo Truong <[email protected]> Removed Root file since it's not needed anymore (opensearch-project#279) Signed-off-by: Theo Truong <[email protected]> Fixed missing global params (opensearch-project#280) Signed-off-by: Theo Truong <[email protected]> Added validation for _info.yaml (opensearch-project#281) DRY'ed JSON schema validation logic Signed-off-by: Theo Truong <[email protected]> Implement inline object schema validator (opensearch-project#282) * Implement inline object schema validator and underlying visitor pattern Signed-off-by: Thomas Farr <[email protected]> * Fix spec lint error Signed-off-by: Thomas Farr <[email protected]> --------- Signed-off-by: Thomas Farr <[email protected]> oops fix lint

Fix: replace pull_request_target with a download/upload artifact.

e06ce32

Signed-off-by: dblock <[email protected]>

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 <[email protected]>

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