Skip to content

Commit 88bd523

Browse files
authored
docs: improve gha vulnerabilities documentation (#645)
* docs: improve gha vulnerabilities documentation * docs: fix code block * docs: add content * docs: review suggestions and vale errors
1 parent 1d83c34 commit 88bd523

File tree

6 files changed

+105
-56
lines changed

6 files changed

+105
-56
lines changed

doc/Makefile

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,9 @@ help:
1919
@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
2020

2121

22-
# customized clean due to examples gallery
22+
# customized clean
2323
clean:
24-
rm -rf build
25-
rm -rf source/examples/
24+
rm -rf _build
2625

2726
# customized pdf fov svg format images
2827
pdf:

doc/source/how-to/compatibility.rst

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ introduced in an earlier version are also supported in later versions. Suppressi
3434
a feature would lead to a backward compatibility issue.
3535

3636
Because the same type of issues can happen with the PyAnsys servers wrapping
37-
Ansys products, creating a similar *maximum version* data structure is
38-
is necessary. While there are no such implementations yet, it should work
39-
in the same way as the minimum version data structure.
37+
Ansys products, creating a similar *maximum version* data structure is necessary.
38+
While there are no such implementations yet, it should work in the same way as the
39+
minimum version data structure.
4040

4141
``version_requires`` decorator
4242
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

doc/source/how-to/vulnerabilities.rst

Lines changed: 82 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ action is up to date and that it is being used in all PyAnsys repositories consi
144144
that the action is implemented correctly and that the results are reviewed regularly.
145145

146146

147-
Addressing common vulnerabilities in python libraries and applications
147+
Addressing common vulnerabilities in Python libraries and applications
148148
----------------------------------------------------------------------
149149

150150
When developing Python applications, it is essential to be aware of common vulnerabilities that can
@@ -365,28 +365,34 @@ For additional examples of fixes, see the `zizmor trophy case`_.
365365

366366
**artipacked**
367367

368+
The vulnerability is that using ``actions/checkout`` in GitHub Actions can store repository credentials in ``.git/config``,
369+
which may be unintentionally exposed through artifacts or workflow steps.
370+
371+
Fixing is important because leaked credentials could grant attackers unauthorized access to your repositories,
372+
which can allow them push malicious code, among other things. See `artipacked audit rule`_ for more information.
373+
368374
.. tab-set::
369375

370376

371-
.. tab-item:: Before
377+
.. tab-item:: Potential risk
372378

373379
.. code:: yaml
374380
375381
steps:
376382
377-
- name: "Checkout project"
383+
- name: "Checkout project" # actions/checkout persists git credentials by default.
378384
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
379385
380386
381-
.. tab-item:: After
387+
.. tab-item:: Remediation
382388

383389
.. code:: yaml
384390
385391
steps:
386392
387393
- name: "Checkout project"
388394
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
389-
with:
395+
with: # Unless needed for git operations in subsequent steps, do not persist credentials.
390396
persist-credentials: false
391397
392398
.. note::
@@ -396,50 +402,64 @@ For additional examples of fixes, see the `zizmor trophy case`_.
396402

397403
**unpinned-uses**
398404

405+
The vulnerability is that using unpinned ``uses:`` clauses in GitHub Actions allows workflows to pull in action
406+
code that can change at any time, including through branch or tag updates.
407+
408+
Fixing it is important because unpinned actions could be modified by attackers or upstream maintainers, leading
409+
to unexpected or malicious code execution in your workflows. See `unpinned-uses audit rule`_ for more
410+
information.
411+
399412
.. tab-set::
400413

401414

402-
.. tab-item:: Before
415+
.. tab-item:: Potential risk
403416

404417
.. code:: yaml
405418
406419
steps:
407420
408421
- name: "Upload distribution artifacts to GitHub artifacts"
409-
uses: actions/upload-artifact@v4
422+
uses: actions/upload-artifact@v4 # The commit a tag-pinned action points to can change due to various factors.
410423
with:
411424
name: ${{ env.LIBRARY_NAME }}-artifacts
412425
path: ~/${{ env.LIBRARY_NAME }}/dist/
413426
414427
415-
.. tab-item:: After
428+
.. tab-item:: Remediation
416429

417430
.. code:: yaml
418431
419432
steps:
420433
421434
- name: "Upload distribution artifacts to GitHub artifacts"
422-
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1
435+
uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 # v4.6.1 # Pinning with a SHA prevents this.
423436
with:
424437
name: ${{ env.LIBRARY_NAME }}-artifacts
425438
path: ~/${{ env.LIBRARY_NAME }}/dist/
426439
440+
.. tip::
441+
442+
You can use the `pinact`_ tool to automatically pin versions of actions and reusable workflows.
443+
427444
.. note::
428445

429446
The ``ansys/actions/check-actions-security`` action has a ``trust-ansys-actions`` option that
430447
allows you to use tags for ``ansys/actions``.
431448
When this option is enabled, you only need to pin external actions.
432449

433-
.. tip::
450+
**github-env**
434451

435-
You can use the `pinact`_ tool to automatically pin versions of actions and reusable workflows.
452+
Writing to ``GITHUB_ENV`` or ``GITHUB_PATH`` in workflows with dangerous triggers (such as ``pull_request_target`` and
453+
``workflow_run``) can let attackers inject arbitrary environment variables / variable contents.
436454

437-
**github-env**
455+
A fix is required because this exposure could allow attackers to run malicious code in your GitHub Actions workflows
456+
either implictly in subsequent steps, or by shadowing ordinary system executables (such as ``ssh``). See
457+
`github-env audit rule`_ for more information.
438458

439459
.. tab-set::
440460

441461

442-
.. tab-item:: Before
462+
.. tab-item:: Potential risk
443463

444464
.. code:: yaml
445465
@@ -449,10 +469,9 @@ For additional examples of fixes, see the `zizmor trophy case`_.
449469
shell: bash
450470
run: |
451471
if [[ ${{ github.ref_name }} =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
452-
# Split the tag into its components
453472
IFS='.' read -ra PARTS <<< "${{ github.ref_name }}"
454-
echo "V_AND_MAJOR=${PARTS[0]}" >> $GITHUB_ENV
455-
echo "MINOR=${PARTS[1]}" >> $GITHUB_ENV
473+
echo "V_AND_MAJOR=${PARTS[0]}" >> $GITHUB_ENV # When used in workflows with dangerous triggers, such as pull_request_target
474+
echo "MINOR=${PARTS[1]}" >> $GITHUB_ENV # and workflow_run, GITHUB_ENV and GITHUB_PATH can be an arbitrary code execution risk.
456475
echo "PATCH=${PARTS[2]}" >> $GITHUB_ENV
457476
else
458477
echo "Invalid tag format. Expected vX.Y.Z but got ${{ github.ref_name }}"
@@ -462,7 +481,6 @@ For additional examples of fixes, see the `zizmor trophy case`_.
462481
- name: "Check tag is valid for current branch"
463482
shell: bash
464483
run: |
465-
# Remove leading "v" from env.X
466484
V_AND_MAJOR=${{ env.V_AND_MAJOR }}
467485
MAJOR="${V_AND_MAJOR#v}"
468486
echo "MAJOR=${MAJOR}" >> $GITHUB_ENV
@@ -494,7 +512,7 @@ For additional examples of fixes, see the `zizmor trophy case`_.
494512
git push origin v${{ env.MAJOR }}
495513
496514
497-
.. tab-item:: After
515+
.. tab-item:: Remediation
498516

499517
.. code:: yaml
500518
@@ -505,11 +523,10 @@ For additional examples of fixes, see the `zizmor trophy case`_.
505523
shell: bash
506524
run: |
507525
if [[ ${{ github.ref_name }} =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
508-
# Split the tag into its components
509526
IFS='.' read -ra PARTS <<< "${{ github.ref_name }}"
510-
echo "V_AND_MAJOR=${PARTS[0]}" >> $GITHUB_OUTPUT
511-
echo "MINOR=${PARTS[1]}" >> $GITHUB_OUTPUT
512-
echo "PATCH=${PARTS[2]}" >> $GITHUB_OUTPUT
527+
echo "V_AND_MAJOR=${PARTS[0]}" >> $GITHUB_OUTPUT # Writing to GITHUB_OUTPUT is safe.
528+
echo "MINOR=${PARTS[1]}" >> $GITHUB_OUTPUT # Writing to GITHUB_OUTPUT is safe.
529+
echo "PATCH=${PARTS[2]}" >> $GITHUB_OUTPUT # Writing to GITHUB_OUTPUT is safe.
513530
else
514531
echo "Invalid tag format. Expected vX.Y.Z but got ${{ github.ref_name }}"
515532
exit 1
@@ -519,10 +536,9 @@ For additional examples of fixes, see the `zizmor trophy case`_.
519536
id: current-branch-tag-validity
520537
shell: bash
521538
env:
522-
V_AND_MAJOR: ${{ steps.tag-components.outputs.V_AND_MAJOR }}
523-
MINOR: ${{ steps.tag-components.outputs.MINOR }}
539+
V_AND_MAJOR: ${{ steps.tag-components.outputs.V_AND_MAJOR }} # Then share information between steps
540+
MINOR: ${{ steps.tag-components.outputs.MINOR }} # through the env block.
524541
run: |
525-
# Remove leading "v" from env.X
526542
MAJOR="${V_AND_MAJOR#v}"
527543
echo "MAJOR=${MAJOR}" >> $GITHUB_OUTPUT
528544
if [[ ${{ github.event.base_ref }} != "refs/heads/release/${MAJOR}.${MINOR}" ]]; then
@@ -567,10 +583,16 @@ For additional examples of fixes, see the `zizmor trophy case`_.
567583

568584
**template-injection**
569585

586+
The vulnerability is that template expansions (``${{ ... }}``) in GitHub Actions can allow code injection when used with
587+
attacker-controlled inputs, such as issue titles (``github.event.issue.title`` which the attacker can fully control by supplying a new issue title).
588+
589+
Fixing it is important because malicious inputs could execute unintended commands, compromising the security of your workflows. See
590+
`template-injection audit rule`_ for more information.
591+
570592
.. tab-set::
571593

572594

573-
.. tab-item:: Before
595+
.. tab-item:: Potential risk
574596

575597
.. code:: yaml
576598
@@ -598,12 +620,12 @@ For additional examples of fixes, see the `zizmor trophy case`_.
598620
599621
- name: "Inspect context variables and workflow input"
600622
run: |
601-
echo ${{ github.workspace }}
602-
echo ${{ runner.temp }}
603-
echo ${{ input.user-input }}
623+
echo ${{ github.workspace }} # Template expansions are resolved before workflows and jobs run. These expansions
624+
echo ${{ runner.temp }} # insert their results directly into the context, which can accidentally introduce shell injection risks.
625+
echo ${{ input.user-input }} # This is especially through when such expansion is from a user input.
604626
605627
606-
.. tab-item:: After
628+
.. tab-item:: Remediation
607629

608630
.. code:: yaml
609631
@@ -631,27 +653,33 @@ For additional examples of fixes, see the `zizmor trophy case`_.
631653
632654
- name: "Inspect context variables and workflow input"
633655
env:
634-
USER_INPUT: ${{ inputs.user-input }}
656+
USER_INPUT: ${{ inputs.user-input }} # Expand inputs and relevant context variables in the env block.
635657
run: |
636-
echo ${USER_INPUT}
637-
echo ${RUNNER_TEMP}
638-
echo ${GITHUB_WORKSPACE}
658+
echo ${USER_INPUT} # Then use that directly within the run block.
659+
echo ${RUNNER_TEMP} # Also, most Github context variables have equivalent environment variables
660+
echo ${GITHUB_WORKSPACE} # that can be directly used in place of template expansions.
639661
640662
.. note::
641663

642664
Notice that ``RUNNER_TEMP`` and ``GITHUB_WORKSPACE`` were not explicitly set in the ``env`` block.
643665
Some GitHub context variables automatically map to environment variables, such as
644-
``runner.temp`` to ``RUNNER_TEMP`` and ``github.workspace`` to ``GITHUB_WORKSPACE``
666+
``runner.temp`` to ``RUNNER_TEMP`` and ``github.workspace`` to ``GITHUB_WORKSPACE``.
645667

646668
If a corresponding environment variable is not automatically available, you must set it in the ``env``
647669
block of the job or step where it is needed before you can use it.
648670

649671
**excessive-permissions**
650672

673+
The vulnerability is that workflows with excessive permissions grant more access than needed, either at the
674+
workflow or job level, including through the default ``GITHUB_TOKEN``.
675+
676+
Fixing it is important because over-scoped permissions increase the risk that a compromised workflow could
677+
perform unauthorized actions on your repository. See `excessive-permissions audit rule`_ for more information.
678+
651679
.. tab-set::
652680

653681

654-
.. tab-item:: Before
682+
.. tab-item:: Potential risk
655683

656684
.. code:: yaml
657685
@@ -669,6 +697,10 @@ For additional examples of fixes, see the `zizmor trophy case`_.
669697
MAIN_PYTHON_VERSION: '3.12'
670698
DOCUMENTATION_CNAME: 'actions.docs.ansys.com'
671699
700+
# When not specified, the default permission assigned to workflows might be too excessive
701+
# for what the jobs need to do. Furthermore, all job steps automatically inherit this
702+
# default permission
703+
672704
concurrency:
673705
group: ${{ github.workflow }}-${{ github.ref }}
674706
cancel-in-progress: true
@@ -698,7 +730,7 @@ For additional examples of fixes, see the `zizmor trophy case`_.
698730
bot-email: ${{ secrets.PYANSYS_CI_BOT_EMAIL }}
699731
700732
701-
.. tab-item:: After
733+
.. tab-item:: Remediation
702734

703735
.. code:: yaml
704736
@@ -716,7 +748,8 @@ For additional examples of fixes, see the `zizmor trophy case`_.
716748
MAIN_PYTHON_VERSION: '3.12'
717749
DOCUMENTATION_CNAME: 'actions.docs.ansys.com'
718750
719-
permissions: {}
751+
permissions: {} # Zero permissions can be granted at the workflow level if not all jobs require permissions.
752+
# As a good rule of thumb, this normally includes jobs that don't use secrets.
720753
721754
concurrency:
722755
group: ${{ github.workflow }}-${{ github.ref }}
@@ -739,7 +772,7 @@ For additional examples of fixes, see the `zizmor trophy case`_.
739772
runs-on: ubuntu-latest
740773
needs: [doc-build]
741774
permissions:
742-
contents: write
775+
contents: write # The specific permission type needed is set for a job that actually needs it.
743776
steps:
744777
- uses: ansys/actions/[email protected]
745778
with:
@@ -750,14 +783,20 @@ For additional examples of fixes, see the `zizmor trophy case`_.
750783
751784
**anonymous-definition**
752785

786+
This issue is raised when workflows omit the ``name:`` field. When ``name:`` is omitted, the workflow is rendered
787+
anonymously in the Github Actions UI, making it harder to understand which definition is running.
788+
789+
There is no security impact associated with this issue. However, it is good practice to always include the ``name:``
790+
field. See `anonymous-definition audit rule`_ for more information.
791+
753792
.. tab-set::
754793

755794

756-
.. tab-item:: Before
795+
.. tab-item:: Potential risk
757796

758797
.. code:: yaml
759798
760-
on: push
799+
on: push # This workflow has no name.
761800
762801
jobs:
763802
build:
@@ -766,11 +805,11 @@ For additional examples of fixes, see the `zizmor trophy case`_.
766805
- run: echo "Hello!"
767806
768807
769-
.. tab-item:: After
808+
.. tab-item:: Remediation
770809

771810
.. code:: yaml
772811
773-
name: Echo Test
812+
name: Echo Test # It is good practice to always name workflows.
774813
on: push
775814
776815
jobs:
@@ -779,7 +818,6 @@ For additional examples of fixes, see the `zizmor trophy case`_.
779818
steps:
780819
- run: echo "Hello!"
781820
782-
783821
Ignoring ``zizmor`` findings
784822
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
785823

doc/source/links.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,12 @@
124124
.. _zizmor audit rules: https://docs.zizmor.sh/audits/
125125
.. _zizmor trophy case: https://docs.zizmor.sh/trophy-case/
126126
.. _ignoring zizmor results: https://docs.zizmor.sh/usage/#ignoring-results
127+
.. _artipacked audit rule: https://docs.zizmor.sh/audits/#artipacked
128+
.. _unpinned-uses audit rule: https://docs.zizmor.sh/audits/#unpinned-uses
129+
.. _github-env audit rule: https://docs.zizmor.sh/audits/#github-env
130+
.. _template-injection audit rule: https://docs.zizmor.sh/audits/#template-injection
131+
.. _excessive-permissions audit rule: https://docs.zizmor.sh/audits/#excessive-permissions
132+
.. _anonymous-definition audit rule: https://docs.zizmor.sh/audits/#anonymous-definition
127133
.. _pinact: https://github.com/suzuki-shunsuke/pinact
128134

129135

0 commit comments

Comments
 (0)