diff --git a/.github/workflows/test-breaking.yaml b/.github/workflows/test-breaking.yaml index 80fc579..a0b8244 100644 --- a/.github/workflows/test-breaking.yaml +++ b/.github/workflows/test-breaking.yaml @@ -341,3 +341,30 @@ jobs: echo "Expected output '$OASDIFF_ACTION_TEST_EXPECTED_OUTPUT' (include-checks must be ignored, not forwarded) but got '$output'" >&2 exit 1 fi + + oasdiff_breaking_unresolved_ref: + runs-on: ubuntu-latest + name: Test a failed comparison is not reported as clean + steps: + - name: checkout + uses: actions/checkout@v7 + - name: Run breaking action against a spec that cannot be loaded + id: test_unresolved_ref + continue-on-error: true + uses: ./breaking + with: + base: 'specs/unresolved-ref.yaml' + revision: 'specs/unresolved-ref.yaml' + review: false + - name: Assert the step failed and reported no clean result + run: | + if [ "${{ steps.test_unresolved_ref.outcome }}" != "failure" ]; then + echo "Expected the action to fail when the spec cannot be loaded" >&2 + exit 1 + fi + case "${{ steps.test_unresolved_ref.outputs.breaking }}" in + *"No breaking changes"*) + echo "A failed comparison must never be reported as 'No breaking changes'" >&2 + exit 1 + ;; + esac diff --git a/.github/workflows/test-diff.yaml b/.github/workflows/test-diff.yaml index c398e74..c9f4a30 100644 --- a/.github/workflows/test-diff.yaml +++ b/.github/workflows/test-diff.yaml @@ -122,3 +122,29 @@ jobs: # oasdiff-token, or a reachable oasdiff-service. # --------------------------------------------------------------------- + + oasdiff_diff_unresolved_ref: + runs-on: ubuntu-latest + name: Test a failed comparison is not reported as clean + steps: + - name: checkout + uses: actions/checkout@v7 + - name: Run diff action against a spec that cannot be loaded + id: test_unresolved_ref + continue-on-error: true + uses: ./diff + with: + base: 'specs/unresolved-ref.yaml' + revision: 'specs/unresolved-ref.yaml' + - name: Assert the step failed and reported no clean result + run: | + if [ "${{ steps.test_unresolved_ref.outcome }}" != "failure" ]; then + echo "Expected the action to fail when the spec cannot be loaded" >&2 + exit 1 + fi + case "${{ steps.test_unresolved_ref.outputs.diff }}" in + *"No changes"*) + echo "A failed comparison must never be reported as 'No changes'" >&2 + exit 1 + ;; + esac diff --git a/.github/workflows/test-validate.yaml b/.github/workflows/test-validate.yaml index 1ac46ce..9ec00ad 100644 --- a/.github/workflows/test-validate.yaml +++ b/.github/workflows/test-validate.yaml @@ -84,3 +84,32 @@ jobs: echo "Expected the step to fail with fail-on WARN" >&2 exit 1 fi + + oasdiff_validate_unresolved_ref: + runs-on: ubuntu-latest + name: Test a failed validation is not reported as clean + steps: + - name: checkout + uses: actions/checkout@v7 + - name: Run validate action against a spec that cannot be loaded + id: test_unresolved_ref + continue-on-error: true + uses: ./validate + with: + spec: 'specs/unresolved-ref.yaml' + - name: Assert the step failed and published no finding counts + run: | + if [ "${{ steps.test_unresolved_ref.outcome }}" != "failure" ]; then + echo "Expected the action to fail when the spec cannot be loaded" >&2 + exit 1 + fi + # A spec that never loaded produces no findings to count. Publishing + # 0 would read as a clean spec to anything gating on these. + if [ "${{ steps.test_unresolved_ref.outputs.findings }}" = "0" ]; then + echo "A failed validation must not report findings=0" >&2 + exit 1 + fi + if [ "${{ steps.test_unresolved_ref.outputs.error_count }}" = "0" ]; then + echo "A failed validation must not report error_count=0" >&2 + exit 1 + fi diff --git a/breaking/entrypoint.sh b/breaking/entrypoint.sh index a636528..a06fa86 100755 --- a/breaking/entrypoint.sh +++ b/breaking/entrypoint.sh @@ -193,13 +193,18 @@ breaking_changes=$(oasdiff breaking "$base" "$revision" $flags $fail_on_flag 2>" # Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is # success and exit 1 is the intended "breaking changes found" / fail-on result; # only codes >=2 (load/parse/etc.) are real errors worth surfacing here. -if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then - echo "::error::$(tr '\n' ' ' < "$_err")" -fi -# Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, -# not message text). Surface the action-specific remedy. -if [ "$exit_code" -eq 123 ]; then - echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff action step." +# Stop here on a real failure: the comparison never ran, so there is nothing to +# report about it. Falling through would report a clean result for a run that +# failed. Matches the changelog action. +if [ "$exit_code" -ge 2 ]; then + [ -s "$_err" ] && echo "::error::$(tr '\n' ' ' < "$_err")" + # Exit code 123 = oasdiff refused a disallowed external $ref (stable + # contract, not message text). Surface the action-specific remedy. + if [ "$exit_code" -eq 123 ]; then + echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff action step." + fi + rm -f "$_err" + exit "$exit_code" fi rm -f "$_err" diff --git a/diff/entrypoint.sh b/diff/entrypoint.sh index 7b4e183..9f9ab9a 100755 --- a/diff/entrypoint.sh +++ b/diff/entrypoint.sh @@ -91,13 +91,18 @@ fi # Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is # success and exit 1 is the intended fail-on-diff result; only codes >=2 # (load/parse/etc.) are real errors worth surfacing here. -if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then - echo "::error::$(tr '\n' ' ' < "$_err")" -fi -# Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, -# not message text). Surface the action-specific remedy. -if [ "$exit_code" -eq 123 ]; then - echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff action step." +# Stop here on a real failure: the comparison never ran, so there is nothing to +# report about it. Falling through would report a clean result for a run that +# failed. Matches the changelog action. +if [ "$exit_code" -ge 2 ]; then + [ -s "$_err" ] && echo "::error::$(tr '\n' ' ' < "$_err")" + # Exit code 123 = oasdiff refused a disallowed external $ref (stable + # contract, not message text). Surface the action-specific remedy. + if [ "$exit_code" -eq 123 ]; then + echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff action step." + fi + rm -f "$_err" + exit "$exit_code" fi rm -f "$_err" diff --git a/specs/unresolved-ref.yaml b/specs/unresolved-ref.yaml new file mode 100644 index 0000000..7adaf2e --- /dev/null +++ b/specs/unresolved-ref.yaml @@ -0,0 +1,11 @@ +# A spec whose $ref cannot be resolved, so oasdiff fails to load it. +# Used to check that a failed comparison is never reported as a clean result. +openapi: 3.0.0 +info: + title: unresolved-ref + version: 1.0.0 +paths: {} +components: + schemas: + Missing: + $ref: './no-such-file.yaml#/Missing' diff --git a/validate/entrypoint.sh b/validate/entrypoint.sh index 331c144..68850f5 100755 --- a/validate/entrypoint.sh +++ b/validate/entrypoint.sh @@ -38,13 +38,18 @@ oasdiff validate $flags --format githubactions "$spec" 2>"$_err" || exit_code=$? # Promote a genuine oasdiff failure to a Checks-tab annotation. Exit 0 is # success and exit 1 is the intended fail-on result; only codes >=2 # (load/parse/etc.) are real errors worth surfacing here. -if [ "$exit_code" -ge 2 ] && [ -s "$_err" ]; then - echo "::error::$(tr '\n' ' ' < "$_err")" -fi -# Exit code 123 = oasdiff refused a disallowed external $ref (stable contract, -# not message text). Surface the action-specific remedy. -if [ "$exit_code" -eq 123 ]; then - echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff validate step." +if [ "$exit_code" -ge 2 ]; then + [ -s "$_err" ] && echo "::error::$(tr '\n' ' ' < "$_err")" + # Exit code 123 = oasdiff refused a disallowed external $ref (stable + # contract, not message text). Surface the action-specific remedy. + if [ "$exit_code" -eq 123 ]; then + echo "::error::oasdiff: this spec resolves external \$refs, which are disabled by default to prevent SSRF on untrusted pull requests. If the spec is trusted, set 'allow-external-refs: true' on the oasdiff validate step." + fi + rm -f "$_err" + # Stop here. Run 2 below would fail the same way and print nothing, and the + # count that reads it cannot tell that from a valid spec, so the outputs + # would say findings=0 and error_count=0 for a spec that never loaded. + exit "$exit_code" fi rm -f "$_err"