harden: default allow-external-refs to false in the actions#128
Merged
Conversation
CI actions run unattended on pull-request content, including from forks, so the safe default for untrusted input is to not resolve external $refs. All five actions now default allow-external-refs to false: - breaking, changelog, diff, pr-comment: new `allow-external-refs` input (default false), threaded through each entrypoint as --allow-external-refs=<value> so the explicit input is authoritative. - validate: input default flipped true -> false. Users whose specs reference external URLs, or who load split multi-file specs by file path rather than the recommended git-ref form, set allow-external-refs: true to opt back in. Specs loaded via the recommended `origin/main:openapi.yaml` git-ref form (single- and multi-file) are unaffected: relative refs resolve via `git show`, not as external refs. README inputs tables updated. NOTE: effective only on an oasdiff base image that honors the flag on the git-ref load path (oasdiff #974, shipping in v1.18.1). Must land together with the Dockerfile bump to v1.18.1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With allow-external-refs defaulting to false, a spec that resolves an
external $ref now fails the step. oasdiff prints the cause to stderr
(visible in logs), but that does not explain the new default or name
the action input, and the file-path load path emits only kin-openapi's
terse "disallowed external reference" with no remedy.
Each entrypoint now captures oasdiff's stderr, still surfaces it, and on
the external-ref failure signature emits a GitHub ::error:: annotation
with the fix: set 'allow-external-refs: true' on the step if the spec is
trusted. This turns a post-upgrade red X into a one-glance fix on the
PR's Checks tab. Detection matches both the git-ref-path message
("external $ref not allowed") and the file-path message ("disallowed
external reference"). No change to the success path or to the
authoritative exit code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e text oasdiff returns a dedicated exit code (123) for a spec that resolves an external $ref under --allow-external-refs=false. Key the recovery hint off that code instead of grepping oasdiff's stderr for "external $ref not allowed" / "disallowed external reference" — text is brittle (kin-openapi owns the file-path wording; messages can change), an exit code is a stable contract. Requires the v1.18.1 base image (oasdiff exit-code change). The raw error is still surfaced for every failure; this only gates the action-specific remedy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft — do not merge until the prerequisites below are met.
What
Makes all five actions default
allow-external-refstofalse, the safe posture for CI that runs unattended on pull-request (incl. fork) content, and adds a recovery hint so the new default is low-friction:Default flip + threading.
breaking,changelog,diff,pr-commentgain anallow-external-refsinput (defaultfalse), threaded through each entrypoint as--allow-external-refs=<value>so the explicit action input is authoritative over any.oasdiff.yamlvalue.validate's input default flipstrue→false. README inputs tables updated.Recovery hint. With external refs off by default, a spec that resolves an external
$refnow fails the step. Each entrypoint captures oasdiff's stderr, still surfaces it, and on the external-ref failure emits a GitHub::error::annotation with the fix:This turns a post-upgrade red ❌ into a one-glance fix on the PR's Checks tab. Detection matches both failure messages: the git-ref-path
external $ref not allowedand the file-pathdisallowed external reference(the latter is kin-openapi's terser text with no remedy of its own). Success path and authoritative exit codes are unchanged.Compatibility
Specs loaded via the recommended git-ref form (
origin/main:openapi.yaml), single- and multi-file, are unaffected — intra-repo relative$refs resolve viagit show, not as external refs. Verified empirically.Opt back in with
allow-external-refs: trueif you either:$ref, orRoot specs given as a URL source (
base: https://…) are not affected — the flag governs$refresolution, not the root load.pr-comment note
pr-commentfails closed on a blocked external$ref(no review token is created, so no comment is posted) and surfaces the same::error::recovery annotation as the other actions. If we later want it to post the remedy as an actual PR comment (its native surface), that's a follow-up — not in this PR.Prerequisites before this can merge / release
tufin/oasdiff:v1.18.1.Until the base image honors the flag on the git-ref load path (oasdiff #974),
--allow-external-refs=falseis a no-op there, so shipping this earlier would give a false sense of protection.Test plan
test-*.yamlworkflows pass (their specs are self-contained / URL-sourced without external$refs, so unaffected).$reffails closed by default, emits the::error::recovery annotation, and passes withallow-external-refs: true.