fix(dereference): cache poisoning when dereferencing external schemas

Author: erunion

lib/dereference.ts

@@ -221,7 +221,27 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
     // If the cached object however is _not_ circular and there are additional keys alongside our
     // `$ref` pointer here we should merge them back in and return that.
     if (cache.circular) {
-      return cache;
+      // If both our cached value and our incoming `$ref` are the same then we can return what we
+      // got out of the cache, otherwise we should re-process this value. We need to do this because
+      // the current dereference caching mechanism doesn't take into account that `$ref` are neither
+      // unique or reference the same file.
+      //
+      // For example if `schema.yaml` references `definitions/child.yaml` and
+      // `definitions/parent.yaml` references `child.yaml` then `$ref: 'child.yaml'` may get cached
+      // for `definitions/child.yaml`, resulting in `schema.yaml` being having an invalid reference
+      // to `child.yaml`.
+      //
+      // This check is not perfect and the design of the dereference caching mechanism needs a total
+      // overhaul.
+      if (typeof cache.value === 'object' && '$ref' in cache.value && '$ref' in $ref) {
+        if (cache.value.$ref === $ref.$ref) {
+          return cache;
+        } else {
+          // no-op
+        }
+      } else {
+        return cache;
+      }
     }
 
     const refKeys = Object.keys($ref);
@@ -238,8 +258,6 @@ function dereference$Ref<S extends object = JSONSchema, O extends ParserOptions<
         value: Object.assign({}, cache.value, extraKeys),
       };
     }
-
-    return cache;
   }
 
   const pointer = $refs._resolve($refPath, path, options);

test/specs/circular-external/circular-external.spec.ts

@@ -53,6 +53,28 @@ describe("Schema with circular (recursive) external $refs", () => {
     expect(schema.definitions.child.properties.parents.items).to.equal(schema.definitions.parent);
   });
 
+  it('should throw an error if "options.dereference.circular" is "ignore"', async () => {
+    const parser = new $RefParser();
+
+    const schema = await parser.dereference(path.rel("test/specs/circular-external/circular-external.yaml"), {
+      dereference: { circular: 'ignore' },
+    });
+
+    expect(schema).to.equal(parser.schema);
+    expect(schema).not.to.deep.equal(dereferencedSchema);
+    expect(parser.$refs.circular).to.equal(true);
+    // @ts-expect-error TS(2532): Object is possibly 'undefined'.
+    expect(schema.definitions.pet.title).to.equal('pet');
+    // @ts-expect-error TS(2532): Object is possibly 'undefined'.
+    expect(schema.definitions.thing).to.deep.equal({ $ref: 'circular-external.yaml#/definitions/thing'});
+    // @ts-expect-error TS(2532): Object is possibly 'undefined'.
+    expect(schema.definitions.person).to.deep.equal({ $ref: 'definitions/person.yaml'});
+    // @ts-expect-error TS(2532): Object is possibly 'undefined'.
+    expect(schema.definitions.parent).to.deep.equal({ $ref: 'definitions/parent.yaml'});
+    // @ts-expect-error TS(2532): Object is possibly 'undefined'.
+    expect(schema.definitions.child).to.deep.equal({ $ref: 'definitions/child.yaml'});
+  });
+
   it('should throw an error if "options.dereference.circular" is false', async () => {
     const parser = new $RefParser();