From d520ec23565c6da012ead7d68b5546827663e974 Mon Sep 17 00:00:00 2001
From: Jon Ursenbach <jon@ursenba.ch>
Date: Fri, 11 Apr 2025 17:35:36 -0700
Subject: [PATCH 1/2] fix(dereference): cache poisoning when dereferencing
 external schemas

---
 lib/dereference.ts                            | 24 ++++++++++++++++---
 .../circular-external.spec.ts                 | 22 +++++++++++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/lib/dereference.ts b/lib/dereference.ts
index 725f703b..66cd392b 100644
--- a/lib/dereference.ts
+++ b/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);
diff --git a/test/specs/circular-external/circular-external.spec.ts b/test/specs/circular-external/circular-external.spec.ts
index 4cdf62f6..169fb02b 100644
--- a/test/specs/circular-external/circular-external.spec.ts
+++ b/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();
 

From 96762189f1d9b91142274b3af2c98f99c7cb5100 Mon Sep 17 00:00:00 2001
From: Jon Ursenbach <jon@ursenba.ch>
Date: Fri, 11 Apr 2025 17:44:54 -0700
Subject: [PATCH 2/2] fix: restructuring some of this confusing cache logic

---
 lib/dereference.ts | 66 ++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/lib/dereference.ts b/lib/dereference.ts
index 66cd392b..40faf7fd 100644
--- a/lib/dereference.ts
+++ b/lib/dereference.ts
@@ -220,43 +220,45 @@ 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) {
-      // 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
+    if (!cache.circular) {
+      const refKeys = Object.keys($ref);
+      if (refKeys.length > 1) {
+        const extraKeys = {};
+        for (const key of refKeys) {
+          if (key !== "$ref" && !(key in cache.value)) {
+            // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
+            extraKeys[key] = $ref[key];
+          }
         }
-      } else {
-        return cache;
+        return {
+          circular: cache.circular,
+          value: Object.assign({}, cache.value, extraKeys),
+        };
       }
+
+      return cache;
     }
 
-    const refKeys = Object.keys($ref);
-    if (refKeys.length > 1) {
-      const extraKeys = {};
-      for (const key of refKeys) {
-        if (key !== "$ref" && !(key in cache.value)) {
-          // @ts-expect-error TS(7053): Element implicitly has an 'any' type because expre... Remove this comment to see the full error message
-          extraKeys[key] = $ref[key];
-        }
+    // 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
       }
-      return {
-        circular: cache.circular,
-        value: Object.assign({}, cache.value, extraKeys),
-      };
+    } else {
+      return cache;
     }
   }