Skip to content

Adjust names of security schemes to match prefixed name #3861

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
StratusFearMe21 wants to merge 6 commits into
base: main
Choose a base branch
from
+102 −2
Open

Adjust names of security schemes to match prefixed name #3861

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73a8153
Adjust names of security schemes to match prefixed name
StratusFearMe21 May 9, 2026
91ab2b0
Add regression test for security schema merging bug
StratusFearMe21 May 11, 2026
526eeba
Remove debug console.dir from regression test
StratusFearMe21 May 15, 2026
e7cf98b
fix: restore inline JSON Schema object fallback in getResolvedInput
pullfrog[bot] May 15, 2026
7339164
chore: trim leftover blank lines from regression test
pullfrog[bot] May 15, 2026
7579098
Merge branch 'main' into fix-merged-security-schemes
mrlubos May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions packages/json-schema-ref-parser/src/__tests__/bundle.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,91 @@ describe('bundle', () => {
expect(pathKeys).toHaveLength(1);
});

it('prefixes operation-level security scheme names with source prefix', async () => {
const refParser = new $RefParser();
const spec1 = {
components: {
securitySchemes: {
bearerAuth: {
bearerFormat: 'JWT',
scheme: 'bearer',
type: 'http',
},
},
},
info: { title: 'Spec 1', version: '1.0.0' },
openapi: '3.0.0',
paths: {
'/users': {
get: {
operationId: 'listUsers',
responses: { '200': { description: 'OK' } },
security: [{ bearerAuth: [] }],
},
},
},
};
const spec2 = {
components: {
securitySchemes: {
bearerAuth: {
bearerFormat: 'JWT',
scheme: 'bearer',
type: 'http',
},
oauth2: {
flows: {
authorizationCode: {
authorizationUrl: 'https://example.com/oauth/authorize',
scopes: { read: 'read access', write: 'write access' },
tokenUrl: 'https://example.com/oauth/token',
},
},
type: 'oauth2',
},
},
},
info: { title: 'Spec 2', version: '1.0.0' },
openapi: '3.0.0',
paths: {
'/orders': {
get: {
operationId: 'listOrders',
responses: { '200': { description: 'OK' } },
security: [{ oauth2: ['read'] }, { bearerAuth: [] }],
},
},
},
};

const merged = (await refParser.bundleMany({
pathOrUrlOrSchemas: [
{ ...spec1, $id: 'file:///spec1.json' },
{ ...spec2, $id: 'file:///spec2.json' },
],
})) as any;

const spec1Prefix = 'spec1';
const spec2Prefix = 'spec2';

console.dir(merged, { depth: null });
Comment thread
StratusFearMe21 marked this conversation as resolved.
Outdated

// Component security scheme names should be prefixed
expect(merged.components.securitySchemes[`${spec1Prefix}_bearerAuth`]).toBeDefined();
expect(merged.components.securitySchemes[`${spec2Prefix}_bearerAuth`]).toBeDefined();
expect(merged.components.securitySchemes[`${spec2Prefix}_oauth2`]).toBeDefined();

// Operation-level security references should also be prefixed
const usersSecurity = merged.paths['/users'].get.security;
expect(usersSecurity).toEqual([{ [`${spec1Prefix}_bearerAuth`]: [] }]);

const ordersSecurity = merged.paths['/orders'].get.security;
expect(ordersSecurity).toEqual([
{ [`${spec2Prefix}_oauth2`]: ['read'] },
{ [`${spec2Prefix}_bearerAuth`]: [] },
]);
});

it('adds prefix to path when HTTP methods conflict', async () => {
const refParser = new $RefParser();
const spec1 = {
Expand Down
24 changes: 19 additions & 5 deletions packages/json-schema-ref-parser/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,20 @@ export function getResolvedInput({
resolvedInput.path = url.fromFileSystemPath(resolvedInput.path);
resolvedInput.type = 'file';
} else if (!resolvedInput.path && pathOrUrlOrSchema && typeof pathOrUrlOrSchema === 'object') {
if ('$id' in pathOrUrlOrSchema && pathOrUrlOrSchema.$id) {
if (
Copy link
Copy Markdown
Member

@mrlubos mrlubos May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StratusFearMe21 why did you need to add this piece? I see the test fails without it, but I'm concerned about tying internals to OpenAPI/Swagger like this

Copy link
Copy Markdown
Author

@StratusFearMe21 StratusFearMe21 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That piece is so that the regression test can set resolvedInput.path on each input schema without the type of the resolvedInput being set to 'url'. This is so that when the test calls bundleMany on the input, the prefixes on the output securitySchemas is predictable.

Copy link
Copy Markdown
Member

@mrlubos mrlubos May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand, is this just to satisfy the test suite? Or is this needed for a real world usage?

Copy link
Copy Markdown
Author

@StratusFearMe21 StratusFearMe21 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it there to satisfy the test suite, but if you were using this package in the real world, passing multiple JSON objects to bundleMany would be broken without this change since both schemas would resolve to the same path . Either url.cwd() or 'schema'.

Copy link
Copy Markdown
Member

@mrlubos mrlubos May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to handle that as a separate issue. I'm also concerned about this package, it feels like it's doing too much too poorly because I'm sure there are other similar edge cases we just didn't run into yet

Copy link
Copy Markdown
Author

@StratusFearMe21 StratusFearMe21 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why $id on a schema object is essential to bundleMany

Line 45 is in getResolvedInput, which is called by parseMany (line 250) — the first step of bundleMany. The critical logic is at lines 44-59:

When a JSON object (not a file path or URL) is passed as an input, resolvedInput.path starts as '' (line 30). Without a $id, the path stays empty, and line 62-64 kicks in:

if (resolvedInput.type !== 'json') {
  resolvedInput.path = url.resolve(url.cwd(), resolvedInput.path);
}

This means a schema without $id falls through entirely — type stays 'url', no path is set, and it gets resolved against url.cwd() (the current working directory).

But when the schema does have $id (and is an OpenAPI/Swagger document), lines 51-52 set resolvedInput.path to the $id value and type to 'json'. This is critical for bundleMany because:

  1. parseMany (line 295) stores this path in this.schemaManySources[i]:

    this.schemaManySources.push(path && path.length ? path : url.cwd());

  2. mergeMany (line 489-490) uses that source path to compute the prefix used for namespacing all components:

    const sourcePath = this.schemaManySources[i] || `multi://input/${i + 1}`;
const prefix = baseName(sourcePath);

  3. That prefix is then used to rename all components (schemas, parameters, securitySchemes, etc.) to avoid collisions between the multiple inputs:

    const newName = `${prefix}_${name}`;  // e.g., "petstore_Pet" instead of "Pet"

  4. It's also used to rewrite $ref pointers (line 512), prefix operation IDs (line 479), prefix tag names (line 522), and resolve relative external references (line 463).

Without $id, an inline schema object would get path = '', which would make baseName return a generic name (or fall back to the CWD-derived path), causing unpredictable or colliding prefixes when multiple JSON objects are bundled together. The $id provides a stable, unique identity for each inline schema, enabling the entire namespacing and reference-rewriting machinery of bundleMany to work correctly.')

('openapi' in pathOrUrlOrSchema && pathOrUrlOrSchema.openapi) ||
('swagger' in pathOrUrlOrSchema && pathOrUrlOrSchema.swagger)
) {
resolvedInput.schema = pathOrUrlOrSchema;
resolvedInput.type = 'json';
if ('$id' in pathOrUrlOrSchema && pathOrUrlOrSchema.$id)
resolvedInput.path = pathOrUrlOrSchema.$id as string;
} else if ('$id' in pathOrUrlOrSchema && pathOrUrlOrSchema.$id) {
// when schema id has defined an URL should use that hostname to request the references,
// instead of using the current page URL
const { hostname, protocol } = new URL(pathOrUrlOrSchema.$id as string);
resolvedInput.path = `${protocol}//${hostname}:${protocol === 'https:' ? 443 : 80}`;
resolvedInput.type = 'url';
} else {
resolvedInput.schema = pathOrUrlOrSchema;
resolvedInput.type = 'json';
}
Comment thread
pullfrog[bot] marked this conversation as resolved.
}

Expand Down Expand Up @@ -385,7 +390,8 @@ export class $RefParser {
const baseName = (p: string) => {
try {
const withoutHash = p.split('#')[0]!;
const parts = withoutHash.split('/');
const withoutTrailingSlash = withoutHash.replace(/\/+$/, '');
const parts = withoutTrailingSlash.split('/');
Copy link
Copy Markdown
Member

@mrlubos mrlubos May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StratusFearMe21 why is this change needed? Reverting it doesn't fail tests

Copy link
Copy Markdown
Author

@StratusFearMe21 StratusFearMe21 May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I didn't mean to keep that in this PR, that's my mistake.

It fixes another bug where if the path on the resolvedInput is set to url.cwd(), because that function always puts a trailing slash at the end of the path, splitting the path /foo/bar/baz/ for example would return ['foo', 'bar', 'baz', '']. Because the last element in parts is empty, baseName would just return 'schema' instead of the last component of the path.

Copy link
Copy Markdown
Member

@mrlubos mrlubos May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to open a separate pull request for that?

const filename = parts[parts.length - 1] || 'schema';
const dot = filename.lastIndexOf('.');
const raw = dot > 0 ? filename.substring(0, dot) : filename;
Expand Down Expand Up @@ -461,6 +467,14 @@ export class $RefParser {
}
} else if (k === 'tags' && Array.isArray(v) && v.every((x) => typeof x === 'string')) {
out[k] = v.map((t) => tagMap.get(t) || t);
} else if (k === 'security' && Array.isArray(v)) {
out[k] = v.map((s) => {
const securityScheme: Record<string, any> = {};
for (const [key, value] of Object.entries(s)) {
securityScheme[`${opIdPrefix}_${key}`] = value;
}
return securityScheme;
});
} else if (k === 'operationId' && typeof v === 'string') {
out[k] = unique(usedOpIds, `${opIdPrefix}_${v}`);
} else {
Expand Down
Loading