Skip to content

Conversation

@weiji14
Copy link
Member

@weiji14 weiji14 commented Mar 18, 2025

Description of proposed changes

Zizmor is a static analysis tool for GitHub Actions, designed to help identify security issues.

Output from initial run at https://github.com/GenericMappingTools/pygmt/actions/runs/13934163536/job/38998263248?pr=3861#step:5:59:

Details
  --> .github/workflows/benchmarks.yml:36:9
   |
36 |         - name: Checkout
   |  _________-
37 | |         uses: actions/[email protected]
38 | |         with:
39 | |           # fetch all history so that setuptools-scm works
40 | |           fetch-depth: 0
   | |________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/benchmarks.yml:27:3
   |
27 | /   benchmarks:
28 | |     runs-on: ubuntu-latest
...  |
88 | |           # See https://github.com/CodSpeedHQ/action/issues/65.
89 | |           run: bash -el -c "python -c \"import pygmt; pygmt.show_versions()\"; PYGMT_USE_EXTERNAL_DISPLAY=false python -m pytest -r P --pyargs pygmt --codspeed --override-ini addopts='--verbose'"
   | |                                                                                                                                                                                                    -
   | |____________________________________________________________________________________________________________________________________________________________________________________________________|
   |                                                                                                                                                                                                      this job
   |                                                                                                                                                                                                      default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/cache_data.yaml:38:9
   |
38 |         - name: Checkout
   |  _________-
39 | |         uses: actions/[email protected]
...  |
43 | |
44 | |       # Install Micromamba with conda-forge dependencies
   | |________________________________________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/cache_data.yaml:29:3
   |
29 | /   gmt_cache:
30 | |     name: Cache GMT artifacts
...  |
82 | |             ~/.gmt/gmt_data_server.txt
83 | |             ~/.gmt/gmt_hash_server.txt
   | |                                       -
   | |_______________________________________|
   |                                         this job
   |                                         default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/check-links.yml:25:7
   |
25 |       - name: Checkout the repository
   |  _______-
26 | |       uses: actions/[email protected]
27 | |       with:
28 | |         path: repository
   | |________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/check-links.yml:30:7
   |
30 |       - name: Checkout the documentation
   |  _______-
31 | |       uses: actions/[email protected]
32 | |       with:
33 | |         ref: gh-pages
34 | |         path: documentation
   | |___________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/check-links.yml:19:3
   |
19 | /   check_links:
20 | |     name: Check Links
...  |
77 | |       env:
78 | |         GH_TOKEN: ${{ github.token }}
   | |                                      -
   | |______________________________________|
   |                                        this job
   |                                        default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

info[template-injection]: code injection via template expansion
  --> .github/workflows/check-links.yml:71:7
   |
71 |       - name: Create Issue From File
   |         ---------------------------- info: this step
72 |         if: env.lychee_exit_code != 0
73 | /       run: |
74 | |         cd repository/
75 | |         title="Link Checker Report on ${{ steps.date.outputs.date }}"
76 | |         gh issue create --title "$title" --body-file /tmp/lychee-out.md
   | |_______________________________________________________________________- info: steps.date.outputs.date may expand into attacker-controllable code
   |
   = note: audit confidence → Low

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/ci_docs.yml:71:9
   |
71 |         - name: Checkout
   |  _________-
72 | |         uses: actions/[email protected]
73 | |         with:
74 | |           # fetch all history so that setuptools-scm works
75 | |           fetch-depth: 0
   | |________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/ci_docs.yml:167:9
    |
167 |         - name: Checkout the gh-pages branch
    |  _________-
168 | |         uses: actions/[email protected]
...   |
174 | |           fetch-depth: 0
175 | |         if: (github.event_name == 'release' || github.event_name == 'push') && (matrix.os == 'ubuntu-latest')
    | |_____________________________________________________________________________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
   --> .github/workflows/ci_docs.yml:47:3
    |
 47 | /   docs:
 48 | |     name: ${{ matrix.os }}
...   |
230 | |           echo -e "\nFinished uploading generated files."
231 | |         if: (github.event_name == 'release' || github.event_name == 'push') && (matrix.os == 'ubuntu-latest')
    | |                                                                                                              -
    | |______________________________________________________________________________________________________________|
    |                                                                                                                this job
    |                                                                                                                default permissions used due to no permissions: block
    |
    = note: audit confidence → Medium

error[template-injection]: code injection via template expansion
   --> .github/workflows/ci_docs.yml:161:9
    |
161 |       - name: Upload the HTML ZIP archive and PDF as release assets
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this step
162 |         run: gh release upload ${{ github.ref_name }} doc/_build/pygmt-docs.zip doc/_build/pygmt-docs.pdf
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ github.ref_name may expand into attacker-controllable code
    |
    = note: audit confidence → High

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/ci_doctests.yaml:37:9
   |
37 |         - name: Checkout
   |  _________-
38 | |         uses: actions/[email protected]
...  |
42 | |
43 | |       # Install Micromamba with conda-forge dependencies
   | |________________________________________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/ci_doctests.yaml:22:3
   |
22 | /   test:
23 | |     name: ${{ matrix.os }}
...  |
84 | |       - name: Run doctests
85 | |         run: make doctest PYTEST_EXTRA="-r P"
   | |                                              -
   | |______________________________________________|
   |                                                this job
   |                                                default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/ci_tests.yaml:106:9
    |
106 |         - name: Checkout
    |  _________-
107 | |         uses: actions/[email protected]
108 | |         with:
109 | |           # fetch all history so that setuptools-scm works
110 | |           fetch-depth: 0
    | |________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/ci_tests_dev.yaml:48:9
   |
48 |         - name: Checkout
   |  _________-
49 | |         uses: actions/[email protected]
50 | |         with:
51 | |           # fetch all history so that setuptools-scm works
52 | |           fetch-depth: 0
   | |________________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[artipacked]: credential persistence through GitHub Actions artifacts
   --> .github/workflows/ci_tests_dev.yaml:93:9
    |
 93 |         - name: Checkout the GMT source from ${{ matrix.gmt_git_ref }} branch
    |  _________-
 94 | |         uses: actions/[email protected]
...   |
100 | |       # Build GMT from source on Linux/macOS, script is adapted from
101 | |       # https://github.com/GenericMappingTools/gmt/blob/6.5.0/ci/build-gmt.sh
    | |_____________________________________________________________________________- does not set persist-credentials: false
    |
    = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
   --> .github/workflows/ci_tests_dev.yaml:32:3
    |
 32 | /   test_gmt_dev:
 33 | |     name: ${{ matrix.os }} - GMT ${{ matrix.gmt_git_ref }}
...   |
188 | |           name: artifact-${{ matrix.os }}-GMT-${{ matrix.gmt_git_ref }}
189 | |           path: tmp-test-dir-with-unique-name
    | |                                              -
    | |______________________________________________|
    |                                                this job
    |                                                default permissions used due to no permissions: block
    |
    = note: audit confidence → Medium
   | |                           -
   | |___________________________|
   |                             this job
   |                             default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/style_checks.yaml:26:9
   |
26 |         - name: Checkout
   |  _________-
27 | |         uses: actions/[email protected]
28 | |
29 | |       # Setup Python
   | |____________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/style_checks.yaml:20:3
   |
20 | /   style_check:
21 | |     name: Style Checks
...  |
65 | |             exit $nfiles
66 | |           fi
   | |             -
   | |_____________|
   |               this job
   |               default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

warning[artipacked]: credential persistence through GitHub Actions artifacts
  --> .github/workflows/type_checks.yml:35:9
   |
35 |         - name: Checkout
   |  _________-
36 | |         uses: actions/[email protected]
37 | |
38 | |       # Setup Python
   | |____________________- does not set persist-credentials: false
   |
   = note: audit confidence → Low

warning[excessive-permissions]: overly broad permissions
  --> .github/workflows/type_checks.yml:29:3
   |
29 | /   static_check:
30 | |     name: Static Type Check
...  |
58 | |       - name: Static type check
59 | |         run: make typecheck
   | |                            -
   | |____________________________|
   |                              this job
   |                              default permissions used due to no permissions: block
   |
   = note: audit confidence → Medium

99 findings (62 suppressed): 0 unknown, 1 informational, 2 low, 31 medium, 3 high

References:

Fixes #

Preview:

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash command is:

  • /format: automatically format and lint the code

@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Mar 18, 2025
@weiji14 weiji14 added this to the 0.15.0 milestone Mar 18, 2025
@weiji14 weiji14 self-assigned this Mar 18, 2025
@weiji14 weiji14 force-pushed the pre-commit/zizmor branch from 0862101 to cd2fcd6 Compare March 18, 2025 22:09
@weiji14 weiji14 marked this pull request as ready for review March 18, 2025 22:23
@weiji14 weiji14 added run/benchmark Trigger the benchmark workflow in PRs run/test-gmt-dev Trigger the GMT Dev Tests workflow in PR and removed run/benchmark Trigger the benchmark workflow in PRs labels Mar 18, 2025
Different environment variable syntax for Powershell
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvars64.bat"
cmake -G Ninja .. ^
-DCMAKE_INSTALL_PREFIX=${{ env.GMT_INSTALL_DIR }} ^
-DCMAKE_INSTALL_PREFIX=${env:GMT_INSTALL_DIR} ^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Windows CI is failing, maybe try the one below (xref: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#using-the-env-context-to-access-environment-variable-values):

Suggested change
-DCMAKE_INSTALL_PREFIX=${env:GMT_INSTALL_DIR} ^
-DCMAKE_INSTALL_PREFIX=$env:GMT_INSTALL_DIR ^

Copy link
Member Author

@weiji14 weiji14 Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it's because of the path? It looks set to GMT_INSTALL_DIR: D:\a\_temp/gmt-install-dir with or without the curly braces, same as at https://github.com/GenericMappingTools/pygmt/actions/runs/13889360104/job/38858639159#step:8:21

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Owh, looks like the Cmake build on Windows didn't complete - https://github.com/GenericMappingTools/pygmt/actions/runs/13934908307/job/39000590987#step:8:669

[0/1] Install the project...
CMake Warning (dev) at cmake_install.cmake:5 (set):
  Syntax error in cmake code at

    D:/a/pygmt/pygmt/gmt/build/cmake_install.cmake:5

  when parsing string

    D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}

  syntax error, unexpected cal_SYMBOL, expecting } (30)

  Policy CMP0010 is not set: Bad variable reference syntax is an error.  Run
  "cmake --help-policy CMP0010" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Install configuration: "Release"
CMake Error at src/cmake_install.cmake:36 (file):
  file cannot create directory:
  D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}/lib.  Maybe need
  administrative privileges.
Call Stack (most recent call first):
  cmake_install.cmake:37 (include)


FAILED: CMakeFiles/install.util 
C:\Windows\system32\cmd.exe /C "cd /D D:\a\pygmt\pygmt\gmt\build && "C:\Program Files\CMake\bin\cmake.exe" -P cmake_install.cmake"
ninja: build stopped: subcommand failed.

Copy link
Member Author

@weiji14 weiji14 Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looking at https://github.com/GenericMappingTools/pygmt/actions/runs/13934908307/job/39000590987#step:8:435, there are these lines:

*  Locations:
*  Installing GMT in          : D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}
*  GMT_DATADIR                : D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}/share
*  GMT_DOCDIR                 : D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}/share/doc
*  GMT_MANDIR                 : D:/a/pygmt/pygmt/gmt/build/${env:GMT_INSTALL_DIR}/share/man

So the environment variable replacement isn't working on powershell Edit: wait, we're not using powershell, it seems to be just regular Windows cmd.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weiji14 weiji14 merged commit d44a40f into main Mar 19, 2025
22 of 27 checks passed
@weiji14 weiji14 deleted the pre-commit/zizmor branch March 19, 2025 01:50
@seisman seisman removed the run/test-gmt-dev Trigger the GMT Dev Tests workflow in PR label Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Boring but important stuff for the core devs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants