Skip to content

Commit

Permalink
art: refactor use of input variables
Browse files Browse the repository at this point in the history
This avoids some akward double quotes and string concats.
At the same time this should now also be safe against command injection,
though since only GH users with repo write access can trigger manual
workflow runs, this wasn't an issue in practice.

Note, command injection is still possible if a pull request modifies
the matrix.* constants to something malicious, but anything running
under the pull_request trigger only has read access to repo and no
access to secrets at all anyway, meaning there’s little point in it.
(This is different for the pull_request_target trigger!)
  • Loading branch information
TheOneric committed Dec 7, 2024
1 parent b5d75da commit cee2df5
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 55 deletions.
19 changes: 11 additions & 8 deletions .github/workflows/art-vm-qemu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,23 @@ jobs:

steps:
- name: Checkout Git Repos
env:
LIBASS_REPO: ${{ github.event.inputs.libass_repo }}
LIBASS_REF: ${{ github.event.inputs.libass_ref }}
TESTS_REPO: ${{ github.repository }}
TESTS_REF: ${{ github.ref }}
run: |
if [ -z "${{ github.event.inputs.libass_repo }}" ] ; then
if [ -z "$LIBASS_REPO" ] ; then
LIBASS_REPO="https://github.com/libass/libass.git"
else
LIBASS_REPO="${{ github.event.inputs.libass_repo }}"
fi
# libass
echo "Cloning Libass Repo: $LIBASS_REPO"
git clone --depth=1 "$LIBASS_REPO" libass
cd libass
if [ ! -z "${{ github.event.inputs.libass_ref }}" ] ; then
if [ -n "$LIBASS_REF" ] ; then
echo "Checking out non-default commit..."
git fetch --depth=1 origin "${{ github.event.inputs.libass_ref }}":artci_laref
git fetch --depth=1 origin "$LIBASS_REF":artci_laref
git checkout --force artci_laref
fi
echo "Testing libass commit:"
Expand All @@ -73,10 +76,10 @@ jobs:
echo ""
# regression tests
git clone --depth=1 'https://github.com/${{ github.repository }}.git' libass-tests
git clone --depth=1 https://github.com/"${TESTS_REPO}".git libass-tests
cd libass-tests
if [ x"${{ github.ref }}" != x ] ; then
git fetch --depth=1 origin ${{ github.ref }}:artci_laref
if [ -n "$TESTS_REF" ] ; then
git fetch --depth=1 origin "$TESTS_REF":artci_laref
git checkout --force artci_laref
else
echo "Could not determine ref! Fallback to current master."
Expand Down
19 changes: 11 additions & 8 deletions .github/workflows/art-vm.yml.disabled
Original file line number Diff line number Diff line change
Expand Up @@ -175,20 +175,23 @@ jobs:
esac

- name: Checkout Git Repos
env:
LIBASS_REPO: ${{ github.event.inputs.libass_repo }}
LIBASS_REF: ${{ github.event.inputs.libass_ref }}
TESTS_REPO: ${{ github.repository }}
TESTS_REF: ${{ github.ref }}
run: |
if [ -z "${{ github.event.inputs.libass_repo }}" ] ; then
if [ -z "$LIBASS_REPO" ] ; then
LIBASS_REPO="https://github.com/libass/libass.git"
else
LIBASS_REPO="${{ github.event.inputs.libass_repo }}"
fi

# libass
echo "Cloning Libass Repo: $LIBASS_REPO"
git clone --depth=1 "$LIBASS_REPO" libass
cd libass
if [ ! -z "${{ github.event.inputs.libass_ref }}" ] ; then
if [ -n "$LIBASS_REF" ] ; then
echo "Checking out non-default commit..."
git fetch --depth=1 origin "${{ github.event.inputs.libass_ref }}":artci_laref
git fetch --depth=1 origin "$LIBASS_REF":artci_laref
git checkout --force artci_laref
fi
echo "Testing libass commit:"
Expand All @@ -198,10 +201,10 @@ jobs:
echo ""

# regression tests
git clone --depth=1 'https://github.com/${{ github.repository }}.git' libass-tests
git clone --depth=1 https://github.com/"${TESTS_REPO}".git libass-tests
cd libass-tests
if [ x"${{ github.ref }}" != x ] ; then
git fetch --depth=1 origin ${{ github.ref }}:artci_laref
if [ -n "$TESTS_REF" ] ; then
git fetch --depth=1 origin "$TESTS_REF":artci_laref
git checkout --force artci_laref
else
echo "Could not determine ref! Fallback to current master."
Expand Down
87 changes: 48 additions & 39 deletions .github/workflows/art.yml
Original file line number Diff line number Diff line change
Expand Up @@ -225,48 +225,57 @@ jobs:
- name: Checkout Git Repos
env:
LIBASS_REPO: ${{ github.event.inputs.libass_repo }}
LIBASS_REF: ${{ github.event.inputs.libass_ref }}
TESTS_REPO: ${{ github.repository }}
TESTS_REF: ${{ github.ref }}
run: |
if [ -z "${{ github.event.inputs.libass_repo }}" ] ; then
LIBASS_REPO="https://github.com/libass/libass.git"
else
LIBASS_REPO="${{ github.event.inputs.libass_repo }}"
if [ -z "$LIBASS_REPO" ] ; then
export LIBASS_REPO="https://github.com/libass/libass.git"
fi
sudo sh -c '
set -e
cd "'"$CHR_DIR"'"/home/artci/workarea
# libass
echo "Cloning Libass Repo: '"$LIBASS_REPO"'"
git clone --depth=1 "'"$LIBASS_REPO"'" libass
cd libass
if [ ! -z "'"${{ github.event.inputs.libass_ref }}"'" ] ; then
echo "Checking out non-default commit..."
git fetch --depth=1 origin '"${{ github.event.inputs.libass_ref }}"':artci_laref
git checkout --force artci_laref
fi
echo "Testing libass commit:"
git log -1 --format=%H
cd ..
echo ""
# regression tests
git clone --depth=1 https://github.com/${{ github.repository }}.git libass-tests
cd libass-tests
if [ x"${{ github.ref }}" != x ] ; then
git fetch --depth=1 origin ${{ github.ref }}:artci_laref
git checkout --force artci_laref
else
echo "Could not determine ref! Fallback to current master."
fi
echo "Using testsuite from commit:"
git log -1 --format=%H
cd ..
echo ""
# Fix ownership
sudo chown -R ${{ steps.config.outputs.uid }}:${{ steps.config.outputs.gid }} *
'
sudo env \
CHR_DIR="$CHR_DIR" \
LIBASS_REPO="$LIBASS_REPO" \
LIBASS_REF="$LIBASS_REF" \
TESTS_REPO="$TESTS_REPO" \
TESTS_REF="$TESTS_REF" \
sh -c '
set -e
cd "$CHR_DIR"/home/artci/workarea
# libass
echo "Cloning Libass Repo: $LIBASS_REPO"
git clone --depth=1 "$LIBASS_REPO" libass
cd libass
if [ -n "$LIBASS_REF" ] ; then
echo "Checking out non-default commit..."
git fetch --depth=1 origin "$LIBASS_REF":artci_laref
git checkout --force artci_laref
fi
echo "Testing libass commit:"
git log -1 --format=%H
cd ..
echo ""
# regression tests
git clone --depth=1 https://github.com/"$TESTS_REPO".git libass-tests
cd libass-tests
if [ -n "$TESTS_REF" ] ; then
git fetch --depth=1 origin "$TESTS_REF":artci_laref
git checkout --force artci_laref
else
echo "Could not determine ref! Fallback to current master."
fi
echo "Using testsuite from commit:"
git log -1 --format=%H
cd ..
echo ""
# Fix ownership
sudo chown -R ${{ steps.config.outputs.uid }}:${{ steps.config.outputs.gid }} *
'
- name: Prepare Chroot Jobs
run: |
Expand Down

0 comments on commit cee2df5

Please sign in to comment.