spectral icon indicating copy to clipboard operation
spectral copied to clipboard

new rule: $ref noticed in invalid places

Open XVincentX opened this issue 5 years ago • 7 comments

In both OAS2 and OAS3, $ref is only allowed in certain places. Spectral is resolving all references anywhere, instead of following the rules of the specification.

Consider this document:

swagger: "2.0"
host: localhost
info:
  description: bar
  title: "IO APIs, shared specs"
  version: "0.1"
paths: {}
definitions:  # definitions non può contenere $ref
  $ref: file.yml

This is technically invalid OAS2, since refs is not allowed here. Spectral does not give any feedback, just says this is valid.

I understand and support the use case, however it would be great if I could at least see a warning saying "Yo man, I resolved this for you but you should not do this"

XVincentX avatar Sep 09 '19 11:09 XVincentX

I'm assuming Spectral is resolving the references for me any way and that's why the validation is passing.

Yeah, that's exactly the cause of the issue. oas2-schema and oas3-schema operate on resolved content, therefore they have no idea about $refs at the time they are executed.

P0lip avatar Sep 09 '19 13:09 P0lip

@XVincentX did you actually mean to say "$refs are not allowed in OpenAPI 2 documents" or did you mean to say something else? Because $ref is definitely allowed in OpenAPI v2 documents.

philsturgeon avatar Sep 09 '19 15:09 philsturgeon

I'm sorry I didn't clarify enough — refs are not supported in the definitions section — it must be just a dictionary of Schema Objects

XVincentX avatar Sep 09 '19 15:09 XVincentX

Ah yeah absolutely. That's confused me in the past. Some tools just resolve everything everywhere and some tools only resolve the specific allowed places. If we resolve everything all the time we are going against the spec, furthering confusion, and giving people a false sense of security in their description docs which might not work in other places outside of stoplight.

But if we error we're being to strict, so it sounds like a warning is pretty consistent with how we want to deal with this sort of thing.

philsturgeon avatar Sep 09 '19 16:09 philsturgeon

Any action on this bug? Btw, I think it has to be marked as a bug, because Spectral advertises itself as an OAS linter with built-in OAS v3 rules. It's reasonable of the reader to believe that OAS v3 validation is part of that linting! By resolving $refs in places where it can't be used, Spectral is validating invalid specs. That's a bug. I came here today explicitly to report this bug...

I have a ticket to add a separate validator to our CI workflow due to this bug. :frowning_face:

hilary avatar Dec 15 '20 19:12 hilary

Spectral is a free, open-source project, and you’ve very welcome to send pull requests if you need this done quickly. I’ll be happy to provide some pointers if you need it but the docs should be a good start on working with rules.

philsturgeon avatar Dec 15 '20 20:12 philsturgeon

I created similar rule for AsyncAPI spec based on unresolved (but only main) document https://github.com/stoplightio/spectral/pull/2262 I can extend that rule to be generic to handle also OpenAPI. However I don't know how to "handle" sub documents (referenced) to check if given referenced sections have also valid places where $ref is allowed, e.g.

paths:
  /users:
    $ref: '../resources/users.yaml'

it's valid, but we don't know if ../resources/users.yaml has $refs in proper places. I will check if documentInventory shares some needed data for that.

magicmatatjahu avatar Sep 16 '22 11:09 magicmatatjahu