feat(integrations): add MCP_OAUTH2_GENERIC integrations to the public API - #7529
Draft
hassan254-prog wants to merge 2 commits into
Draft
hassan254-prog wants to merge 2 commits into
hassan254-prog wants to merge 2 commits into
Conversation
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
hassan254-prog
marked this pull request as draft
September 14, 2026 19:16
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found and verified against the latest diff
Confidence score: 3/5
docs/spec.yamlrejects valid MCP responses when optional client metadata is formatted asnull; mark all three properties nullable to keep the public contract aligned with runtime output.packages/server/lib/services/integration.service.tscan silently drop non-emptyintegrationConfigwhen no provider schema exists, and its truthy check prevents clearingoauth_client_uriwith an emptyclient_uri; preserve non-empty config and use presence-based handling for explicit clears.packages/server/lib/services/integration.service.unit.test.tscurrently locks in a likely validation gap by accepting custom STS mode withoutstsEndpointUrl; require the endpoint and assert the error, or remove the characterization test.packages/server/lib/formatters/integration.tsduplicates credential projection logic fromintegrationCredentialsToMcp, creating drift risk between public and MCP outputs; extract a shared domain-to-wire mapping.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/server/lib/services/integration.service.ts">
<violation number="1" location="packages/server/lib/services/integration.service.ts:317">
P2: When a provider has no `integration_config` schema but the request supplies non-empty `integrationConfig`, this condition skips validation and silently drops the values. Keep the block enabled for non-empty input so `resolveIntegrationConfig` returns the existing invalid-configuration error.</violation>
<violation number="2" location="packages/server/lib/services/integration.service.ts:603">
P2: When an existing MCP integration is updated with `client_uri: ''`, this truthy check omits the field and preserves the old `oauth_client_uri`, so callers cannot clear a branding override even though the validation schema accepts the empty value. Check for `!== undefined` (or explicitly delete the stored key for an empty value).</violation>
</file>
<file name="docs/spec.yaml">
<violation number="1" location="docs/spec.yaml:5274">
P2: When an MCP integration omits optional client metadata, the formatter returns `null`, but these properties permit only strings. Mark all three properties nullable so valid responses conform to the public contract.</violation>
</file>
<file name="packages/server/lib/services/integration.service.unit.test.ts">
<violation number="1" location="packages/server/lib/services/integration.service.unit.test.ts:762">
P2: This test codifies a purported validation gap as successful behavior. Either make `stsEndpointUrl` required for `stsMode: custom` and assert an error here, or remove this characterization test so the missing validation is not treated as the contract.</violation>
</file>
<file name="packages/server/lib/formatters/integration.ts">
<violation number="1" location="packages/server/lib/formatters/integration.ts:111">
P3: These new credential projections duplicate the corresponding branches in `integrationCredentialsToMcp`, so the public and MCP outputs can silently drift when either mapping changes. Extract the shared domain-to-wire mapping while keeping the endpoint response schemas separate.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| integration.custom = { | ||
| ...integration.custom, | ||
| ...(credentials.client_name && { oauth_client_name: credentials.client_name }), | ||
| ...(credentials.client_uri && { oauth_client_uri: credentials.client_uri }), |
Contributor
There was a problem hiding this comment.
P2: When an existing MCP integration is updated with client_uri: '', this truthy check omits the field and preserves the old oauth_client_uri, so callers cannot clear a branding override even though the validation schema accepts the empty value. Check for !== undefined (or explicitly delete the stored key for an empty value).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/lib/services/integration.service.ts, line 603:
<comment>When an existing MCP integration is updated with `client_uri: ''`, this truthy check omits the field and preserves the old `oauth_client_uri`, so callers cannot clear a branding override even though the validation schema accepts the empty value. Check for `!== undefined` (or explicitly delete the stored key for an empty value).</comment>
<file context>
@@ -589,6 +595,16 @@ function applyCredentials(integration: DBCreateIntegration, credentials: CreateI
+ integration.custom = {
+ ...integration.custom,
+ ...(credentials.client_name && { oauth_client_name: credentials.client_name }),
+ ...(credentials.client_uri && { oauth_client_uri: credentials.client_uri }),
+ ...(credentials.client_logo_uri && { oauth_client_logo_uri: credentials.client_logo_uri })
+ };
</file context>
Suggested change
| ...(credentials.client_uri && { oauth_client_uri: credentials.client_uri }), | |
| ...(credentials.client_uri !== undefined && { oauth_client_uri: credentials.client_uri }), |
Comment on lines
+5274
to
+5283
| client_name: | ||
| type: string | ||
| minLength: 1 | ||
| maxLength: 255 | ||
| client_uri: | ||
| type: string | ||
| maxLength: 255 | ||
| client_logo_uri: | ||
| type: string | ||
| maxLength: 255 |
Contributor
There was a problem hiding this comment.
P2: When an MCP integration omits optional client metadata, the formatter returns null, but these properties permit only strings. Mark all three properties nullable so valid responses conform to the public contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/spec.yaml, line 5274:
<comment>When an MCP integration omits optional client metadata, the formatter returns `null`, but these properties permit only strings. Mark all three properties nullable so valid responses conform to the public contract.</comment>
<file context>
@@ -5263,3 +5263,47 @@ components:
+ type: string
+ enum:
+ - MCP_OAUTH2_GENERIC
+ client_name:
+ type: string
+ minLength: 1
</file context>
Suggested change
| client_name: | |
| type: string | |
| minLength: 1 | |
| maxLength: 255 | |
| client_uri: | |
| type: string | |
| maxLength: 255 | |
| client_logo_uri: | |
| type: string | |
| maxLength: 255 | |
| client_name: | |
| anyOf: | |
| - type: string | |
| minLength: 1 | |
| maxLength: 255 | |
| - type: 'null' | |
| client_uri: | |
| anyOf: | |
| - type: string | |
| maxLength: 255 | |
| - type: 'null' | |
| client_logo_uri: | |
| anyOf: | |
| - type: string | |
| maxLength: 255 | |
| - type: 'null' |
| ); | ||
| }); | ||
|
|
||
| it('does NOT catch a patch that switches to custom STS mode without the now-required endpoint URL (known gap)', async () => { |
Contributor
There was a problem hiding this comment.
P2: This test codifies a purported validation gap as successful behavior. Either make stsEndpointUrl required for stsMode: custom and assert an error here, or remove this characterization test so the missing validation is not treated as the contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/lib/services/integration.service.unit.test.ts, line 762:
<comment>This test codifies a purported validation gap as successful behavior. Either make `stsEndpointUrl` required for `stsMode: custom` and assert an error here, or remove this characterization test so the missing validation is not treated as the contract.</comment>
<file context>
@@ -596,6 +718,70 @@ describe('integrationService', () => {
+ );
+ });
+
+ it('does NOT catch a patch that switches to custom STS mode without the now-required endpoint URL (known gap)', async () => {
+ const provider = shared.getProvider('aws-sigv4');
+ if (!provider) {
</file context>
| const resolvedConfig = resolveIntegrationConfig(provider, params.integrationConfig); | ||
| if (resolvedConfig.isErr()) { | ||
| if (params.custom && Object.keys(params.custom).length > 0) { | ||
| if (provider.integration_config) { |
Contributor
There was a problem hiding this comment.
P2: When a provider has no integration_config schema but the request supplies non-empty integrationConfig, this condition skips validation and silently drops the values. Keep the block enabled for non-empty input so resolveIntegrationConfig returns the existing invalid-configuration error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/lib/services/integration.service.ts, line 317:
<comment>When a provider has no `integration_config` schema but the request supplies non-empty `integrationConfig`, this condition skips validation and silently drops the values. Keep the block enabled for non-empty input so `resolveIntegrationConfig` returns the existing invalid-configuration error.</comment>
<file context>
@@ -307,30 +313,30 @@ export class IntegrationService {
- const resolvedConfig = resolveIntegrationConfig(provider, params.integrationConfig);
- if (resolvedConfig.isErr()) {
+ if (params.custom && Object.keys(params.custom).length > 0) {
+ if (provider.integration_config) {
return Err(
new IntegrationServiceError({
</file context>
Suggested change
| if (provider.integration_config) { | |
| if (provider.integration_config || (params.integrationConfig && Object.keys(params.integrationConfig).length > 0)) { |
| app_link: credentials.appLink, | ||
| private_key: credentials.privateKey | ||
| }; | ||
| case 'MCP_OAUTH2_GENERIC': |
Contributor
There was a problem hiding this comment.
P3: These new credential projections duplicate the corresponding branches in integrationCredentialsToMcp, so the public and MCP outputs can silently drift when either mapping changes. Extract the shared domain-to-wire mapping while keeping the endpoint response schemas separate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/lib/formatters/integration.ts, line 111:
<comment>These new credential projections duplicate the corresponding branches in `integrationCredentialsToMcp`, so the public and MCP outputs can silently drift when either mapping changes. Extract the shared domain-to-wire mapping while keeping the endpoint response schemas separate.</comment>
<file context>
@@ -108,5 +108,14 @@ export function integrationCredentialsToPublicApi(credentials: IntegrationCreden
app_link: credentials.appLink,
private_key: credentials.privateKey
};
+ case 'MCP_OAUTH2_GENERIC':
+ return {
+ type: credentials.type,
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 5/5
- In
docs/llms-full.txt, theaws-sigv4description inaccurately implies thatintegration_configreplaces fixed credential fields, which could mildly mislead readers about provider configuration; clarify that it may be declared alongside or instead of fixed credentials.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/llms-full.txt">
<violation number="1" location="docs/llms-full.txt:9781">
P3: `aws-sigv4` declares both fixed credentials and `integration_config`, so “instead of fixed credentials fields” is inaccurate. Say that these providers may declare `integration_config` alongside or instead of fixed credential fields, and clarify that this response contains only its integration-config values.
(Based on your team's feedback about unsettable implementation details in customer-facing API docs.) [03d645a1-2ced-4991-ba87-71ded0c2abfb]</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| </ResponseExample> | ||
|
|
||
| <Info> | ||
| For providers that declare an `integration_config` schema instead of fixed credentials fields (e.g. `aws-sigv4`, `epic`, `private-api-generic`), `credentials` returns `type: "INTEGRATION_CONFIG"` with the provider's actual `auth_mode` and its `integration_config` values -- only the fields that provider's schema declares, nothing else from the integration's internal storage. |
Contributor
There was a problem hiding this comment.
P3: aws-sigv4 declares both fixed credentials and integration_config, so “instead of fixed credentials fields” is inaccurate. Say that these providers may declare integration_config alongside or instead of fixed credential fields, and clarify that this response contains only its integration-config values.
(Based on your team's feedback about unsettable implementation details in customer-facing API docs.) [03d645a1-2ced-4991-ba87-71ded0c2abfb]
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/llms-full.txt, line 9781:
<comment>`aws-sigv4` declares both fixed credentials and `integration_config`, so “instead of fixed credentials fields” is inaccurate. Say that these providers may declare `integration_config` alongside or instead of fixed credential fields, and clarify that this response contains only its integration-config values.
(Based on your team's feedback about unsettable implementation details in customer-facing API docs.) [03d645a1-2ced-4991-ba87-71ded0c2abfb]</comment>
<file context>
@@ -9731,8 +9731,56 @@ Source: https://nango.dev/docs/reference/backend/http-api/integration/get.md
</ResponseExample>
+<Info>
+For providers that declare an `integration_config` schema instead of fixed credentials fields (e.g. `aws-sigv4`, `epic`, `private-api-generic`), `credentials` returns `type: "INTEGRATION_CONFIG"` with the provider's actual `auth_mode` and its `integration_config` values -- only the fields that provider's schema declares, nothing else from the integration's internal storage.
+</Info>
+
</file context>
Suggested change
| For providers that declare an `integration_config` schema instead of fixed credentials fields (e.g. `aws-sigv4`, `epic`, `private-api-generic`), `credentials` returns `type: "INTEGRATION_CONFIG"` with the provider's actual `auth_mode` and its `integration_config` values -- only the fields that provider's schema declares, nothing else from the integration's internal storage. | |
| For providers that declare an `integration_config` schema, alongside or instead of fixed credential fields (e.g. `aws-sigv4`, `epic`, `private-api-generic`), `credentials` returns `type: "INTEGRATION_CONFIG"` with the provider's actual `auth_mode` and its `integration_config` values -- only the fields that provider's schema declares, nothing else from the integration's internal storage. |
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| integrationConfig[field] = value; | ||
| } | ||
| } | ||
| return { type: 'INTEGRATION_CONFIG', authMode: provider.auth_mode, integration_config: integrationConfig }; |
There was a problem hiding this comment.
P1: The public integration response now exposes provider-declared integration configuration values, including secrets
The new response branch returns every provider-declared custom value, including secret-bearing integration fields, through the public API.
Allowlist non-sensitive fields and mask/exclude secrets before formatting the public integration response.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="packages/server/lib/utils/integrations.ts">
<violation number="1" location="packages/server/lib/utils/integrations.ts:91">
<priority>P1</priority>
<title>The public integration response now exposes provider-declared integration configuration values, including secrets</title>
<evidence>The new INTEGRATION_CONFIG branch iterates over every key declared by provider.integration_config and returns each corresponding custom value in integration_config. Provider schemas can declare credential fields such as AWS secret access keys, so the public formatter can return decrypted credential material to API callers; the added GET documentation also demonstrates secret-shaped AWS credential values in this response. Encrypting configuration at rest does not prevent this response-time disclosure.</evidence>
<recommendation>Do not return secret-bearing integration_config fields from the public API. Add an explicit per-field sensitivity/allowlist policy that excludes secrets and returns only non-sensitive configuration (or masked/presence indicators), update the formatter and OpenAPI schema accordingly, and replace the documentation example with non-secret values.</recommendation>
</violation>
</file>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the problem and your solution