fix: OpenAPI import fails when securitySchemes are not defined#6429
Conversation
WalkthroughReworks getSecurity(apiSpec) in the OpenAPI→Bruno converter: removes an early-return, computes whether schemes exist, builds supported schemes by mapping default schemes to securitySchemes via the first key, filters falsy entries, and simplifies getScheme to a single-line direct lookup (securitySchemes[schemeName]). Adds two OpenAPI fixtures and a Playwright import test suite. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/import/openapi/security-schemes-import.spec.ts (2)
10-16: LGTM! Proper test structure.The test correctly validates the import of OpenAPI specs with security schemes, using appropriate fixtures and cleanup.
Consider wrapping the import logic in
test.stepfor improved test reporting:test('Import OpenAPI spec with security schemes', async ({ page, createTmpDir }) => { - const openApiFile = path.resolve(__dirname, 'fixtures', 'openapi-with-security-schemes.json'); - - await importCollection(page, openApiFile, await createTmpDir('openapi-with-security'), { - expectedCollectionName: 'API with Security Schemes' - }); + await test.step('Import OpenAPI spec with security schemes', async () => { + const openApiFile = path.resolve(__dirname, 'fixtures', 'openapi-with-security-schemes.json'); + + await importCollection(page, openApiFile, await createTmpDir('openapi-with-security'), { + expectedCollectionName: 'API with Security Schemes' + }); + }); });
18-24: LGTM! Proper test structure.The test correctly validates the import of OpenAPI specs without security schemes, covering the key scenario this PR fixes.
Consider wrapping the import logic in
test.stepfor improved test reporting:test('Import OpenAPI spec without security schemes', async ({ page, createTmpDir }) => { - const openApiFile = path.resolve(__dirname, 'fixtures', 'openapi-without-security-schemes.json'); - - await importCollection(page, openApiFile, await createTmpDir('openapi-without-security'), { - expectedCollectionName: 'API without Security Schemes' - }); + await test.step('Import OpenAPI spec without security schemes', async () => { + const openApiFile = path.resolve(__dirname, 'fixtures', 'openapi-without-security-schemes.json'); + + await importCollection(page, openApiFile, await createTmpDir('openapi-without-security'), { + expectedCollectionName: 'API without Security Schemes' + }); + }); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(2 hunks)tests/import/openapi/fixtures/openapi-with-security-schemes.json(1 hunks)tests/import/openapi/fixtures/openapi-without-security-schemes.json(1 hunks)tests/import/openapi/security-schemes-import.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-converters/src/openapi/openapi-to-bruno.jstests/import/openapi/security-schemes-import.spec.ts
tests/**/**.*
⚙️ CodeRabbit configuration file
tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:
Follow best practices for Playwright code and e2e automation
Try to reduce usage of
page.waitForTimeout();in code unless absolutely necessary and the locator cannot be found using existingexpect()playwright callsAvoid using
page.pause()in codeUse locator variables for locators
Avoid using test.only
Use multiple assertions
Promote the use of
test.stepas much as possible so the generated reports are easier to readEnsure that the
fixtureslike the collections are nested inside thefixturesfolderFixture Example*: Here's an example of possible fixture and test pair
. ├── fixtures │ └── collection │ ├── base.bru │ ├── bruno.json │ ├── collection.bru │ ├── ws-test-request-with-headers.bru │ ├── ws-test-request-with-subproto.bru │ └── ws-test-request.bru ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection ├── headers.spec.ts ├── persistence.spec.ts ├── variable-interpolation │ ├── fixtures │ │ └── collection │ │ ├── environments │ │ ├── bruno.json │ │ └── ws-interpolation-test.bru │ ├── init-user-data │ └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection └── subproto.spec.ts
Files:
tests/import/openapi/fixtures/openapi-without-security-schemes.jsontests/import/openapi/security-schemes-import.spec.tstests/import/openapi/fixtures/openapi-with-security-schemes.json
🧠 Learnings (3)
📚 Learning: 2025-12-02T07:24:50.311Z
Learnt from: bijin-bruno
Repo: usebruno/bruno PR: 6263
File: packages/bruno-requests/src/auth/oauth2-helper.ts:249-249
Timestamp: 2025-12-02T07:24:50.311Z
Learning: In OAuth2 Basic Auth headers for Bruno, clientSecret is optional and can be omitted. When constructing the Authorization header in `packages/bruno-requests/src/auth/oauth2-helper.ts`, use `clientSecret || ''` instead of `clientSecret!` to properly handle cases where only clientId is provided, per community requests.
Applied to files:
packages/bruno-converters/src/openapi/openapi-to-bruno.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.test.{js,jsx,ts,tsx} : Add tests for any new functionality or meaningful changes. If code is added, removed, or significantly modified, corresponding tests should be updated or created
Applied to files:
tests/import/openapi/security-schemes-import.spec.ts
📚 Learning: 2025-12-16T07:16:08.934Z
Learnt from: sanish-bruno
Repo: usebruno/bruno PR: 6090
File: tests/scripting/hooks/init-user-data/ui-state-snapshot.json:1-8
Timestamp: 2025-12-16T07:16:08.934Z
Learning: For e2e tests in the bruno repository: Collections that are shared between CLI and UI tests (comprehensive test suites testing core functionality) should be placed in `packages/bruno-tests/` to avoid duplication. The `tests/**/fixtures/collection` pattern should be used for test-specific collections that test particular UI behaviors or are specific to a single test file.
Applied to files:
tests/import/openapi/security-schemes-import.spec.ts
🧬 Code graph analysis (1)
tests/import/openapi/security-schemes-import.spec.ts (2)
tests/utils/page/actions.ts (1)
closeAllCollections(715-715)packages/bruno-converters/src/openapi/openapi-to-bruno.js (1)
path(309-309)
🪛 Checkov (3.2.334)
tests/import/openapi/fixtures/openapi-without-security-schemes.json
[high] 1-291: Ensure that the global security field has rules defined
(CKV_OPENAPI_4)
[high] 1-291: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 57-62: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
tests/import/openapi/fixtures/openapi-with-security-schemes.json
[high] 1-208: Ensure that security operations is not empty.
(CKV_OPENAPI_5)
[medium] 69-85: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
[high] 26-29: Ensure that security schemes don't allow cleartext credentials over unencrypted channel - version 3.x.y files
(CKV_OPENAPI_3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: CLI Tests
- GitHub Check: Unit Tests
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
🔇 Additional comments (4)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (2)
932-934: LGTM! Properly handles missing security schemes.The addition of
getScheme: () => nullwhen security schemes are absent ensures the method is always defined, preventing the "getScheme is not a function" error.
944-944: LGTM! Safe fallback for missing schemes.The
|| nullfallback ensures a consistent return type when a scheme name doesn't exist insecuritySchemes. Calling code at line 381 safely checks for null before using the result.tests/import/openapi/fixtures/openapi-with-security-schemes.json (1)
1-208: LGTM! Well-structured test fixture.This fixture appropriately covers multiple security scheme types (bearer, basic, API key, OAuth2) and different operation-level security requirements, providing comprehensive test coverage for the security schemes import functionality.
tests/import/openapi/fixtures/openapi-without-security-schemes.json (1)
1-291: LGTM! Appropriate test fixture for the no-security-schemes scenario.This fixture correctly represents an OpenAPI spec without any security schemes defined, which is the exact scenario the PR fixes. The comprehensive endpoint coverage ensures the converter handles all operation types when security schemes are absent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (1)
926-941: Minor:getSchemereturnsundefinedinstead ofnullas specified in PR description.The PR objective states:
getScheme: () => nullwhen schemes emptygetScheme: (name) => securitySchemes[name] || nullwhen schemes existCurrent implementation returns
undefinedfor missing schemes. While functionally equivalent (both falsy), returningnullis more explicit and matches the stated behavior.- getScheme: (schemeName) => securitySchemes[schemeName] + getScheme: (schemeName) => securitySchemes[schemeName] || null
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CODING_STANDARDS.md)
**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions.() => {}is good
No space between function name and parentheses.func()notfunc ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly
Files:
packages/bruno-converters/src/openapi/openapi-to-bruno.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: SSL Tests - Linux
- GitHub Check: SSL Tests - Windows
- GitHub Check: SSL Tests - macOS
- GitHub Check: Playwright E2E Tests
- GitHub Check: Unit Tests
- GitHub Check: CLI Tests
🔇 Additional comments (1)
packages/bruno-converters/src/openapi/openapi-to-bruno.js (1)
929-937: Logic looks correct for the fix.The refactored
getSecurityfunction now:
- Computes
hasSchemesto check ifsecuritySchemesis non-empty- Returns an empty
supportedarray when no schemes exist- Always provides a
getSchememethod, preventing the "is not a function" errorThe
.filter(Boolean)correctly handles cases where a default scheme references a non-existent security scheme definition.
BRU-2347
OpenAPI import fails with "e.global.security.getScheme is not a function" when importing specs that don't define securitySchemes.
Cause
The recent PR #6288 fixed a bug in the
getSecurityfunction:Before:
Object.keys(securitySchemes) === 0(always false since arrays ≠ numbers)After:
Object.keys(securitySchemes).length === 0(correctly checks for empty)However, when no
securitySchemesare defined, the function returns{ supported: [] }without agetSchememethod, causing the error when other code tries to callgetScheme().Fix
Ensure
getSchememethod always exists, returningnullwhen no security schemes are defined:securitySchemesis empty:getScheme: () => nullsecuritySchemesexists:getScheme: (name) => securitySchemes[name] || nullSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.