Skip to content

chore(cli): added support for api keys in query params #8892

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

chore(cli): added support for api keys in query params #8892

wants to merge 5 commits into from
+72 −46

Conversation

aditya-arolkar-swe
Copy link
Contributor

@aditya-arolkar-swe aditya-arolkar-swe commented Aug 20, 2025 •
edited
Loading

Description

A client introduced a new security schema:

components:
 securitySchemes:
    APIKeyQueryParam:
      type: apiKey
      name: apiKey
      in: query

However, we didn't support this and the error thrown wasn't clear.

Changes Made

  • Added support for api keys in query param
  • Added an improved error logging for when we fail to convert securitySchemas

Testing

  • Manual testing completed

tjb9dc
tjb9dc reviewed Aug 20, 2025
View reviewed changes
packages/cli/api-importers/openapi/openapi-ir-parser/src/openapi/v3/generateIr.ts Outdated
Comment on lines 71 to 75
if (convertedSecurityScheme == undefined) {
throw new Error(`Security scheme conversion failed for ${key}: ${JSON.stringify(securityScheme)}`);
}
if (convertedSecurityScheme == null) {
return [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's intended to be the difference between this and the == null check below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't thinking about the null conditional block - was just trying to solve the undefined case. What happened was the returned schema came out as undefined, then when we try to access .type in the code just below we get the error in the CLI output as Cannot read properties of undefined (reading 'type'). This was just hoping to be an earlier error pitstop that would clarify that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

caught up sync about the glorious null vs undefined check and == / === in js

tjb9dc
tjb9dc reviewed Aug 20, 2025
View reviewed changes
...i/api-importers/openapi/openapi-ir-parser/src/openapi/v3/converters/convertSecurityScheme.ts Outdated
passwordEnvVar: basicSecuritySchemeNamingAndEnvvar?.password?.env
});
} else {
throw new Error(`Unsupported HTTP scheme: ${securityScheme.scheme}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's log the key in here too

tjb9dc
tjb9dc reviewed Aug 20, 2025
View reviewed changes
...i/api-importers/openapi/openapi-ir-parser/src/openapi/v3/converters/convertSecurityScheme.ts
queryParameterName: securityScheme.name
});
} else {
throw new Error(`Unsupported API key location ${key}: ${securityScheme.in}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I need to dig in at some point about how we surface CLI errors, I feel like there's a juicier way to do this and make it nicely formatted. But this is okay for now I think

@github-actions GitHub Actions
Copy link

This PR is stale because it has been open 25 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale This PR hasn't has any commits or comments in 25 days or more. label Sep 17, 2025
@github-actions GitHub Actions
Copy link

This PR was closed because it has been inactive for 5 days after being marked stale.

@github-actions github-actions bot closed this Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjb9dc tjb9dc tjb9dc approved these changes

Assignees
No one assigned
Labels
Stale This PR hasn't has any commits or comments in 25 days or more.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aditya-arolkar-swe @tjb9dc @fern-support