From 38fc04dc653e06a6e1ec0cbde14f822295208c41 Mon Sep 17 00:00:00 2001 From: Pascal Zimmermann Date: Mon, 26 Oct 2026 19:58:59 +0100 Subject: [PATCH] feat: Harden start.sh argument handling and add unit and e2e tests --- .github/workflows/test.yml | 21 +++++++ README.md | 2 +- action.yml | 2 +- start.js | 2 +- start.sh | 82 ++++++++++++------------- tests/e2e.sh | 97 +++++++++++++++++++++++++++++ tests/lib.sh | 22 +++++++ tests/unit.sh | 122 +++++++++++++++++++++++++++++++++++++ 8 files changed, 303 insertions(+), 47 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100755 tests/e2e.sh create mode 100755 tests/lib.sh create mode 100755 tests/unit.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..35be655 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,21 @@ +name: Tests + +on: + pull_request: + push: + branches: [master, main] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 24 + - name: Lint shell + run: shellcheck start.sh tests/*.sh + - name: Unit tests + run: bash tests/unit.sh + - name: E2E tests + run: bash tests/e2e.sh diff --git a/README.md b/README.md index c7ffa97..6116738 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ jobs: | force_with_lease | boolean | false | Determines if force-with-lease push is used. Please specify the corresponding branch inside `ref` section of the checkout action e.g. `ref: ${{ github.head_ref }}`. Be aware, if you want to update the branch and the corresponding tag please use the `force` parameter instead of the `force_with_lease` option. | | atomic | boolean | true | Determines if atomic push is used. | | push_to_submodules | string | 'on-demand' | Determines if --recurse-submodules= is used. The value defines the used strategy. | -| push_only_tags | boolean | false | Determines if the action should only push the tags, default false | +| push_only_tags | boolean | false | Determines if the action should only push the tags (never the branch), default false | | tags | boolean | false | Determines if `--tags` is used. | | directory | string | '.' | Directory to change to before pushing. | | repository | string | '' | Repository name.
Default or empty repository name represents
current github repository.
If you want to push to other repository,
you should make a [personal access token](https://github.com/settings/tokens)
and use it as the `token` input. | diff --git a/action.yml b/action.yml index f88ca29..bd0cdc5 100644 --- a/action.yml +++ b/action.yml @@ -43,7 +43,7 @@ inputs: description: 'Determines if --tags is used' required: false push_only_tags: - description: 'Determines if the action should only push the tags, default false' + description: 'Determines if the action should only push the tags (never the branch), default false' required: false directory: description: 'Directory to change to before pushing.' diff --git a/start.js b/start.js index 023f456..b799f9a 100644 --- a/start.js +++ b/start.js @@ -68,5 +68,5 @@ const main = async () => { main().catch(err => { console.error(err); - process.exit(-1); + process.exit(1); }) diff --git a/start.sh b/start.sh index bfdafbd..4b17f5a 100644 --- a/start.sh +++ b/start.sh @@ -10,8 +10,6 @@ INPUT_PUSH_ONLY_TAGS=${INPUT_PUSH_ONLY_TAGS:-false} INPUT_DIRECTORY=${INPUT_DIRECTORY:-"."} INPUT_PUSH_TO_SUBMODULES=${INPUT_PUSH_TO_SUBMODULES:-""} INPUT_PULL=${INPUT_PULL:-false} -_ATOMIC_OPTION="" -_FORCE_OPTION="" REPOSITORY=${INPUT_REPOSITORY:-$GITHUB_REPOSITORY} if ${INPUT_PUSH_ONLY_TAGS}; then @@ -26,32 +24,12 @@ fi exit 1; }; -if ${INPUT_FORCE} && ${INPUT_FORCE_WITH_LEASE}; then +if [ "${INPUT_FORCE}" = "true" ] && [ "${INPUT_FORCE_WITH_LEASE}" = "true" ]; then echo "Please, specify only force or force_with_lease and not both."; exit 1; fi -if ${INPUT_ATOMIC}; then - _ATOMIC_OPTION="--atomic" -fi - -if ${INPUT_FORCE}; then - _FORCE_OPTION="--force" -fi - -if ${INPUT_FORCE_WITH_LEASE}; then - _FORCE_OPTION="--force-with-lease" -fi - -if ${INPUT_TAGS} || ${INPUT_PUSH_ONLY_TAGS}; then - _TAGS="--tags" -fi - -if [ -n "${INPUT_PUSH_TO_SUBMODULES}" ]; then - _INPUT_PUSH_TO_SUBMODULES="--recurse-submodules=${INPUT_PUSH_TO_SUBMODULES}" -fi - -cd ${INPUT_DIRECTORY} +cd "${INPUT_DIRECTORY}" if git symbolic-ref --quiet HEAD > /dev/null 2>&1; then _DETACHED_HEAD=false @@ -59,7 +37,7 @@ else _DETACHED_HEAD=true fi -if ${_DETACHED_HEAD}; then +if [ "${_DETACHED_HEAD}" = "true" ]; then echo "Warning: The repository is in a detached HEAD state."; echo "This happens when actions/checkout checks out a tag, a commit SHA, or a"; echo "pull-request merge commit rather than a real branch."; @@ -69,8 +47,22 @@ if ${_DETACHED_HEAD}; then echo "the push destination — it does not check out a branch in your workspace."; fi +# Resolved once and shared by pull and push. The token is not embedded in the +# URL (it would show up in process arguments and git error messages); it is +# handed to git through its environment as a host-scoped auth header instead. +if [ "${INPUT_SSH}" = "true" ]; then + remote_repo="git@${INPUT_GITHUB_URL}:${REPOSITORY}.git" +else + remote_repo="${INPUT_GITHUB_URL_PROTOCOL}//${INPUT_GITHUB_URL}/${REPOSITORY}.git" + _auth=$(printf 'oauth2:%s' "${INPUT_GITHUB_TOKEN}" | base64 | tr -d '\n') + _n=${GIT_CONFIG_COUNT:-0} + export "GIT_CONFIG_KEY_${_n}=http.${INPUT_GITHUB_URL_PROTOCOL}//${INPUT_GITHUB_URL}/.extraheader" + export "GIT_CONFIG_VALUE_${_n}=AUTHORIZATION: basic ${_auth}" + export GIT_CONFIG_COUNT=$((_n + 1)) +fi + if [ "${INPUT_PULL}" != "false" ]; then - if ${_DETACHED_HEAD}; then + if [ "${_DETACHED_HEAD}" = "true" ]; then echo "Error: 'pull' is enabled but the repository is in detached HEAD state." echo "git pull only works when a branch is currently checked out." echo "Either disable the 'pull' input or check out a branch explicitly" @@ -78,12 +70,6 @@ if [ "${INPUT_PULL}" != "false" ]; then exit 1 fi - if ${INPUT_SSH}; then - _pull_remote="git@${INPUT_GITHUB_URL}:${REPOSITORY}.git" - else - _pull_remote="${INPUT_GITHUB_URL_PROTOCOL}//oauth2:${INPUT_GITHUB_TOKEN}@${INPUT_GITHUB_URL}/${REPOSITORY}.git" - fi - case "${INPUT_PULL}" in rebase|true) _pull_strategy="--rebase" @@ -101,23 +87,31 @@ if [ "${INPUT_PULL}" != "false" ]; then esac echo "Pulling from ${INPUT_BRANCH} (strategy: ${INPUT_PULL}) before push..." - git pull ${_pull_strategy} "${_pull_remote}" "${INPUT_BRANCH}" + git pull "${_pull_strategy}" "${remote_repo}" "${INPUT_BRANCH}" fi -if ${INPUT_SSH}; then - remote_repo="git@${INPUT_GITHUB_URL}:${REPOSITORY}.git" -else - remote_repo="${INPUT_GITHUB_URL_PROTOCOL}//oauth2:${INPUT_GITHUB_TOKEN}@${INPUT_GITHUB_URL}/${REPOSITORY}.git" +# Build the push argument list; "$@" keeps each argument intact (no word splitting). +set -- +if [ "${INPUT_PUSH_ONLY_TAGS}" = "true" ]; then + # Never pushes the branch, only tags. + set -- "${remote_repo}" +elif [ "${INPUT_FORCE_WITH_LEASE}" != "true" ]; then + set -- "${remote_repo}" "HEAD:${INPUT_BRANCH}" fi -if ${INPUT_PUSH_ONLY_TAGS}; then - ADDITIONAL_PARAMETERS="${remote_repo}" -elif ! ${INPUT_FORCE_WITH_LEASE}; then - ADDITIONAL_PARAMETERS="${remote_repo} HEAD:${INPUT_BRANCH}" +[ -n "${INPUT_PUSH_TO_SUBMODULES}" ] && set -- "$@" "--recurse-submodules=${INPUT_PUSH_TO_SUBMODULES}" + +# --atomic is incompatible with --force-with-lease combined with --tags. +if [ "${INPUT_ATOMIC}" = "true" ] && ! { [ "${INPUT_FORCE_WITH_LEASE}" = "true" ] && [ "${INPUT_TAGS}" = "true" ]; }; then + set -- "$@" "--atomic" fi -if ${INPUT_FORCE_WITH_LEASE} && ${INPUT_TAGS}; then - _ATOMIC_OPTION="" +set -- "$@" "--follow-tags" + +[ "${INPUT_FORCE}" = "true" ] && set -- "$@" "--force" +[ "${INPUT_FORCE_WITH_LEASE}" = "true" ] && set -- "$@" "--force-with-lease" +if [ "${INPUT_TAGS}" = "true" ] || [ "${INPUT_PUSH_ONLY_TAGS}" = "true" ]; then + set -- "$@" "--tags" fi -git push $ADDITIONAL_PARAMETERS $_INPUT_PUSH_TO_SUBMODULES $_ATOMIC_OPTION --follow-tags $_FORCE_OPTION $_TAGS; +git push "$@" diff --git a/tests/e2e.sh b/tests/e2e.sh new file mode 100755 index 0000000..0acc890 --- /dev/null +++ b/tests/e2e.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# End-to-end tests: run start.js (and thus start.sh) with real git against a +# local bare repository. git's insteadOf rewrites the action's HTTPS URL to it. +ROOT=$(cd "$(dirname "$0")/.." && pwd) +. "$ROOT/tests/lib.sh" + +SANDBOX=$(mktemp -d) +trap 'rm -rf "$SANDBOX"' EXIT +export HOME="$SANDBOX/home"; mkdir -p "$HOME" +export GIT_CONFIG_NOSYSTEM=1 +git config --global user.name tester +git config --global user.email tester@example.test +git config --global init.defaultBranch main +git config --global protocol.file.allow always + +BARE="$SANDBOX/remote.git" +URL="https://example.test/owner/repo.git" +git config --global "url.file://$BARE.insteadOf" "$URL" + +# fresh_world: new bare remote with one commit on main, plus a work clone. +fresh_world() { + rm -rf "$BARE" "$SANDBOX/seed" "$SANDBOX/work" + git init -q --bare "$BARE" + git init -q "$SANDBOX/seed" + (cd "$SANDBOX/seed" && echo base > f && git add f && git commit -qm base \ + && git push -q "$BARE" HEAD:main) + git clone -q "$BARE" "$SANDBOX/work" +} +commit_in() { (cd "$1" && echo "$2" > "$2" && git add "$2" && git commit -qm "$2"); } +remote_head() { git -C "$BARE" rev-parse "${1:-main}"; } + +# action [VAR=value...]: run the action from the work clone. +action() { + OUT=$(cd "$SANDBOX/work" && env INPUT_BRANCH=main INPUT_TOKEN=tok \ + INPUT_GITHUB_URL=https://example.test INPUT_REPOSITORY=owner/repo \ + GITHUB_REPOSITORY=owner/repo "$@" node "$ROOT/start.js" 2>&1) + CODE=$? +} + +echo "plain push" +fresh_world; commit_in "$SANDBOX/work" a +action +assert_eq "exits 0" 0 "$CODE" +assert_eq "remote main updated" "$(git -C "$SANDBOX/work" rev-parse HEAD)" "$(remote_head)" + +echo "non-fast-forward is rejected" +fresh_world; commit_in "$SANDBOX/work" a; commit_in "$SANDBOX/seed" b +(cd "$SANDBOX/seed" && git push -q "$BARE" HEAD:main) +action +assert_eq "exits 1" 1 "$CODE" + +echo "force overwrites diverged remote" +action INPUT_FORCE=true +assert_eq "exits 0" 0 "$CODE" +assert_eq "remote main replaced" "$(git -C "$SANDBOX/work" rev-parse HEAD)" "$(remote_head)" + +echo "pull rebase integrates remote changes" +fresh_world; commit_in "$SANDBOX/work" a; commit_in "$SANDBOX/seed" b +(cd "$SANDBOX/seed" && git push -q "$BARE" HEAD:main) +action INPUT_PULL=rebase +assert_eq "exits 0" 0 "$CODE" +assert_eq "remote has both commits" 3 "$(git -C "$BARE" rev-list --count main)" + +echo "tags" +fresh_world; commit_in "$SANDBOX/work" a; git -C "$SANDBOX/work" tag v1 +action INPUT_TAGS=true +assert_eq "exits 0" 0 "$CODE" +assert_eq "tag pushed" "$(git -C "$SANDBOX/work" rev-parse v1)" "$(git -C "$BARE" rev-parse v1)" + +echo "directory input" +fresh_world; commit_in "$SANDBOX/work" a +mkdir "$SANDBOX/elsewhere" +OUT=$(cd "$SANDBOX/elsewhere" && env INPUT_BRANCH=main INPUT_TOKEN=tok INPUT_GITHUB_URL=https://example.test \ + INPUT_REPOSITORY=owner/repo INPUT_DIRECTORY="$SANDBOX/work" node "$ROOT/start.js" 2>&1); CODE=$? +assert_eq "exits 0" 0 "$CODE" +assert_eq "pushed from directory" "$(git -C "$SANDBOX/work" rev-parse HEAD)" "$(remote_head)" + +echo "push_only_tags" +fresh_world; commit_in "$SANDBOX/work" a; git -C "$SANDBOX/work" tag v2 +action INPUT_PUSH_ONLY_TAGS=true INPUT_TAGS=true +assert_eq "exits 0" 0 "$CODE" +assert_eq "tag pushed" "$(git -C "$SANDBOX/work" rev-parse v2)" "$(git -C "$BARE" rev-parse v2)" +assert_eq "branch not pushed" 1 "$(git -C "$BARE" rev-list --count main)" + +echo "detached HEAD" +fresh_world; git -C "$SANDBOX/work" checkout -q --detach +action +assert_contains "warns" "$OUT" "detached HEAD" +action INPUT_PULL=true +assert_eq "pull on detached HEAD exits 1" 1 "$CODE" + +echo "validation" +fresh_world +action INPUT_TOKEN= INPUT_GITHUB_TOKEN= +assert_eq "missing token exits 1" 1 "$CODE" + +summary diff --git a/tests/lib.sh b/tests/lib.sh new file mode 100755 index 0000000..05c2f32 --- /dev/null +++ b/tests/lib.sh @@ -0,0 +1,22 @@ +#!/bin/bash +# Minimal assertion helpers shared by unit.sh and e2e.sh. +PASS=0 +FAIL=0 + +ok() { PASS=$((PASS + 1)); echo " ok - $1"; } +fail() { FAIL=$((FAIL + 1)); echo " FAIL - $1"; [ -n "${2:-}" ] && echo " $2"; } + +assert_eq() { # name expected actual + if [ "$2" = "$3" ]; then ok "$1"; else fail "$1" "expected '$2', got '$3'"; fi +} +assert_contains() { # name haystack needle + case "$2" in *"$3"*) ok "$1" ;; *) fail "$1" "'$3' not found in: $2" ;; esac +} +assert_not_contains() { # name haystack needle + case "$2" in *"$3"*) fail "$1" "'$3' unexpectedly found in: $2" ;; *) ok "$1" ;; esac +} + +summary() { + echo; echo "passed: $PASS, failed: $FAIL" + [ "$FAIL" -eq 0 ] +} diff --git a/tests/unit.sh b/tests/unit.sh new file mode 100755 index 0000000..7a2c9ea --- /dev/null +++ b/tests/unit.sh @@ -0,0 +1,122 @@ +#!/bin/bash +# Unit tests for start.sh: a fake `git` records its arguments so we can assert +# on option handling without touching a real repository or network. +ROOT=$(cd "$(dirname "$0")/.." && pwd) +. "$ROOT/tests/lib.sh" + +SANDBOX=$(mktemp -d) +trap 'rm -rf "$SANDBOX"' EXIT +mkdir -p "$SANDBOX/bin" +cat > "$SANDBOX/bin/git" <<'FAKE' +#!/bin/sh +echo "$*" >> "$GIT_LOG" +[ -n "$GIT_CONFIG_COUNT" ] && echo "env:$GIT_CONFIG_KEY_0=$GIT_CONFIG_VALUE_0" >> "$GIT_LOG" +if [ "$1" = "symbolic-ref" ]; then + [ "$FAKE_DETACHED" = "true" ] && exit 1 +fi +exit 0 +FAKE +chmod +x "$SANDBOX/bin/git" + +# run_start VAR=value... -> sets OUT (stdout+stderr), CODE, GIT_CALLS (log) +run_start() { + export GIT_LOG="$SANDBOX/git.log" + : > "$GIT_LOG" + OUT=$(cd "$SANDBOX" && env -i PATH="$SANDBOX/bin:$PATH" GIT_LOG="$GIT_LOG" \ + INPUT_BRANCH=main INPUT_GITHUB_TOKEN=tok INPUT_GITHUB_URL_PROTOCOL=https: \ + INPUT_GITHUB_URL=github.com INPUT_REPOSITORY=owner/repo "$@" \ + bash "$ROOT/start.sh" 2>&1) + CODE=$? + GIT_CALLS=$(cat "$GIT_LOG") +} +push_line() { grep '^push' "$GIT_LOG"; } +REMOTE="https://github.com/owner/repo.git" +AUTH="http.https://github.com/.extraheader=AUTHORIZATION: basic $(printf oauth2:tok | base64)" + +echo "defaults" +run_start +assert_eq "exits 0" 0 "$CODE" +assert_eq "default push command" "push $REMOTE HEAD:main --atomic --follow-tags" "$(push_line)" + +echo "token handling" +run_start +assert_not_contains "token is not in any git argument" "$(grep -v '^env:' "$GIT_LOG")" "tok" +assert_contains "token passed via scoped auth header" "$GIT_CALLS" "env:$AUTH" + +echo "validation" +run_start INPUT_GITHUB_TOKEN= +assert_eq "missing token exits 1" 1 "$CODE" +assert_contains "missing token message" "$OUT" "Missing input 'token'" +assert_eq "missing token does not push" "" "$(push_line)" + +run_start INPUT_FORCE=true INPUT_FORCE_WITH_LEASE=true +assert_eq "force + force_with_lease exits 1" 1 "$CODE" +assert_contains "force conflict message" "$OUT" "only force or force_with_lease" + +echo "push options" +run_start INPUT_ATOMIC=false +assert_not_contains "atomic=false drops --atomic" "$(push_line)" "--atomic" + +run_start INPUT_FORCE=true +assert_contains "force adds --force" "$(push_line)" "--force" +assert_not_contains "force is not lease" "$(push_line)" "--force-with-lease" + +run_start INPUT_FORCE_WITH_LEASE=true +assert_contains "lease adds --force-with-lease" "$(push_line)" "--force-with-lease" +assert_not_contains "lease omits HEAD refspec" "$(push_line)" "HEAD:main" +assert_not_contains "lease uses default remote" "$(push_line)" "$REMOTE" + +run_start INPUT_FORCE_WITH_LEASE=true INPUT_PUSH_ONLY_TAGS=true +assert_contains "lease+only_tags pushes to bare remote" "$(push_line)" "push $REMOTE" + +run_start INPUT_PUSH_ONLY_TAGS=true INPUT_TAGS=true +assert_not_contains "only_tags omits HEAD refspec" "$(push_line)" "HEAD:main" +assert_contains "only_tags pushes tags" "$(push_line)" "--tags" + +run_start INPUT_TAGS=true +assert_contains "tags adds --tags" "$(push_line)" "--tags" + +run_start INPUT_FORCE_WITH_LEASE=true INPUT_TAGS=true +assert_not_contains "lease+tags drops --atomic" "$(push_line)" "--atomic" + +run_start INPUT_PUSH_TO_SUBMODULES=on-demand +assert_contains "submodule strategy" "$(push_line)" "--recurse-submodules=on-demand" + +run_start INPUT_SSH=true +assert_contains "ssh remote" "$(push_line)" "git@github.com:owner/repo.git" +assert_not_contains "ssh remote has no token" "$GIT_CALLS" "oauth2" +assert_not_contains "ssh sends no auth header" "$GIT_CALLS" "extraheader" + +run_start INPUT_GITHUB_URL=ghe.example.com INPUT_BRANCH=release +assert_contains "enterprise url + branch" "$(push_line)" "https://ghe.example.com/owner/repo.git HEAD:release" + +echo "detached HEAD" +run_start FAKE_DETACHED=true +assert_eq "detached push still exits 0" 0 "$CODE" +assert_contains "detached warning" "$OUT" "detached HEAD" + +echo "pull" +run_start INPUT_PULL=true +assert_contains "pull=true rebases" "$GIT_CALLS" "pull --rebase $REMOTE main" +run_start INPUT_PULL=rebase +assert_contains "pull=rebase" "$GIT_CALLS" "pull --rebase" +run_start INPUT_PULL=merge +assert_contains "pull=merge" "$GIT_CALLS" "pull --no-rebase" +run_start INPUT_PULL=ff-only +assert_contains "pull=ff-only" "$GIT_CALLS" "pull --ff-only" +run_start INPUT_PULL=bogus +assert_eq "unknown strategy exits 1" 1 "$CODE" +assert_contains "unknown strategy message" "$OUT" "Unknown pull strategy" +assert_eq "unknown strategy does not push" "" "$(push_line)" +run_start INPUT_PULL=true FAKE_DETACHED=true +assert_eq "pull on detached HEAD exits 1" 1 "$CODE" +assert_eq "pull on detached HEAD does not push" "" "$(push_line)" +run_start INPUT_PULL=merge INPUT_SSH=true +assert_contains "pull over ssh" "$GIT_CALLS" "pull --no-rebase git@github.com:owner/repo.git main" + +echo "directory" +mkdir -p "$SANDBOX/sub dir" +run_start INPUT_DIRECTORY="sub dir" +assert_eq "directory with space works" 0 "$CODE" + +summary