diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 82f3a98..b5f2ea4 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -750,3 +750,114 @@ jobs: exit 1 fi echo "PASS" + + pr_comment_free_review_url_preserves_https_base: + runs-on: ubuntu-latest + name: Test pr-comment free review URL keeps the https:// scheme on URL-style base + # Regression test for a bug surfaced by oasdiff-test/test#59: when the + # workflow passes a raw.githubusercontent.com URL as the base input + # (as the integration-test repo does), the entrypoint's + # base_path=$(echo "$base" | sed 's/.*://') + # stripped the "https:" prefix and left a broken "//raw.github..." + # URL. The free /review page then tried to GET that broken URL and + # rendered "access denied" with no useful diagnostic — because the + # generic access-denied screen masked the real cause (malformed URL). + # The fix passes URL-shaped inputs through unchanged. + steps: + - uses: actions/checkout@v6 + - name: Stub oasdiff to a no-changes spec + run: | + set -euo pipefail + mkdir -p /tmp/stub + cat > /tmp/stub/oasdiff <<'STUB' + #!/bin/sh + echo '[]' + STUB + chmod +x /tmp/stub/oasdiff + + mkdir -p /tmp/run + export GITHUB_REF=refs/pull/123/merge + export GITHUB_REPOSITORY=oasdiff-test/test + export GITHUB_SHA=deadbeef + export GITHUB_BASE_REF=main + export GITHUB_STEP_SUMMARY=/tmp/run/step-summary + cat > /tmp/run/event.json <&1) + echo "--- entrypoint output ---" + echo "$out" + + if ! echo "$out" | grep -q "::notice::.*View API changes"; then + echo "FAIL: notice line missing" >&2 + exit 1 + fi + notice=$(echo "$out" | grep "::notice::.*View API changes") + # The encoded base_file= param must contain the full URL, not the + # stripped "//raw..." form. https:// URL-encodes to https%3A%2F%2F. + if echo "$notice" | grep -q 'base_file=https%3A%2F%2Fraw.githubusercontent.com'; then + echo "PASS: full https://raw... URL preserved in base_file" + elif echo "$notice" | grep -q 'base_file=%2F%2Fraw.githubusercontent.com'; then + echo "FAIL: 'https:' was stripped from base — the strip_ref_prefix URL guard is missing" >&2 + echo "notice line: $notice" >&2 + exit 1 + else + echo "FAIL: base_file= param did not contain the raw.githubusercontent.com host as expected" >&2 + echo "notice line: $notice" >&2 + exit 1 + fi + + pr_comment_free_review_url_strips_git_ref_prefix: + runs-on: ubuntu-latest + name: "Test pr-comment free review URL strips origin/main: prefix from git-ref base" + # Companion to pr_comment_free_review_url_preserves_https_base: when + # the base input is git-ref form (origin/main:openapi.yaml), the + # prefix must still be stripped so the /review page receives just the + # path. The previous sed-based implementation handled this correctly; + # this test guards the case-branch refactor against breaking it. + steps: + - uses: actions/checkout@v6 + - name: Stub oasdiff to a no-changes spec + run: | + set -euo pipefail + mkdir -p /tmp/stub + cat > /tmp/stub/oasdiff <<'STUB' + #!/bin/sh + echo '[]' + STUB + chmod +x /tmp/stub/oasdiff + + mkdir -p /tmp/run + export GITHUB_REF=refs/pull/123/merge + export GITHUB_REPOSITORY=foo/bar + export GITHUB_SHA=deadbeef + export GITHUB_BASE_REF=main + export GITHUB_STEP_SUMMARY=/tmp/run/step-summary + cat > /tmp/run/event.json <&1) + echo "--- entrypoint output ---" + echo "$out" + + notice=$(echo "$out" | grep "::notice::.*View API changes") + # base_file should be just "multi-file/openapi.yaml" (URL-encoded slash), + # not the full "origin/main:multi-file/openapi.yaml". + if echo "$notice" | grep -q 'base_file=multi-file%2Fopenapi.yaml'; then + echo "PASS: origin/main: prefix stripped from base" + else + echo "FAIL: base_file= did not have the git-ref prefix stripped" >&2 + echo "notice line: $notice" >&2 + exit 1 + fi diff --git a/pr-comment/entrypoint.sh b/pr-comment/entrypoint.sh index 50f8893..8ab4340 100755 --- a/pr-comment/entrypoint.sh +++ b/pr-comment/entrypoint.sh @@ -77,9 +77,24 @@ repo="${GITHUB_REPOSITORY#*/}" # Emit free review annotation (public repos only — no auth required) urlencode() { printf '%s' "$1" | jq -sRr @uri; } -# Strip git ref prefix (e.g. "origin/main:path.yaml" -> "path.yaml", "HEAD:path.yaml" -> "path.yaml") -base_path=$(echo "$base" | sed 's/.*://') -rev_path=$(echo "$revision" | sed 's/.*://') +# Resolve the on-disk / on-repo path portion of `base` and `revision` for +# the free /review URL. Two input shapes are supported: +# +# 1. Git-ref form — "origin/main:openapi.yaml" or "HEAD:openapi.yaml". +# The sed strips everything up to the colon so we keep just the path. +# 2. URL form — "https://raw.githubusercontent.com/o/r/main/foo.yaml". +# The naive sed would also strip "https:" and leave a broken +# "//raw.githubusercontent.com/..." which the /review page then can't +# fetch (it renders as the access-denied screen with a misleading +# "owner doesn't have access" message). Pass URLs through unchanged. +strip_ref_prefix() { + case "$1" in + http://*|https://*) printf '%s' "$1" ;; + *) printf '%s' "$1" | sed 's/.*://' ;; + esac +} +base_path=$(strip_ref_prefix "$base") +rev_path=$(strip_ref_prefix "$revision") # Prefer the base SHA over the branch name so the link is commit-pinned. # Fall back through `git rev-parse origin/` before resorting to # the branch name itself, so push-event triggers (no pull_request payload)