From 9b7a3a7de96698d85a036cbfe5b1dc8a4a0ad973 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+Gricha@users.noreply.github.com> Date: Wed, 10 Dec 2025 07:20:33 +0000 Subject: [PATCH] Unique names, but simplify it a little bit cause i had conflict --- .../integrations.$integrationId.edit.tsx | 8 +- .../_authRequired+/integrations.new.tsx | 10 +- api/db/migrations-index.ts | 1 + .../20251210_integration_name_unique.ts | 34 +++++++ api/models/integration.ts | 18 +++- api/validation/integration-name.test.ts | 96 +++++++++++++++++++ api/validation/integration-name.ts | 22 +++++ tests/tests/integrations-graphql.test.ts | 40 ++++---- tests/tests/pat-link.test.ts | 10 +- tests/tests/subroutine-integrations.test.ts | 4 +- 10 files changed, 212 insertions(+), 31 deletions(-) create mode 100644 api/migrations/20251210_integration_name_unique.ts create mode 100644 api/validation/integration-name.test.ts create mode 100644 api/validation/integration-name.ts diff --git a/admin-panel/app/routes/_authRequired+/integrations.$integrationId.edit.tsx b/admin-panel/app/routes/_authRequired+/integrations.$integrationId.edit.tsx index 0e57355..b0c3cc0 100644 --- a/admin-panel/app/routes/_authRequired+/integrations.$integrationId.edit.tsx +++ b/admin-panel/app/routes/_authRequired+/integrations.$integrationId.edit.tsx @@ -518,7 +518,13 @@ export default function EditIntegrationPage() { {errors.name &&

{errors.name.message}

} diff --git a/admin-panel/app/routes/_authRequired+/integrations.new.tsx b/admin-panel/app/routes/_authRequired+/integrations.new.tsx index e9b7ca5..1a58933 100644 --- a/admin-panel/app/routes/_authRequired+/integrations.new.tsx +++ b/admin-panel/app/routes/_authRequired+/integrations.new.tsx @@ -633,8 +633,14 @@ export default function NewIntegrationPage() { {errors.name &&

{errors.name.message}

} diff --git a/api/db/migrations-index.ts b/api/db/migrations-index.ts index b976d66..ce78f52 100644 --- a/api/db/migrations-index.ts +++ b/api/db/migrations-index.ts @@ -13,4 +13,5 @@ export const migrations = { "20251127_pat_link": import("../migrations/20251127_pat_link.ts"), "20251127_visibility": import("../migrations/20251127_visibility.ts"), "20251205_0633_mcp_plugin_oauth": import("../migrations/20251205_0633_mcp_plugin_oauth.ts"), + "20251210_integration_name_unique": import("../migrations/20251210_integration_name_unique.ts"), }; diff --git a/api/migrations/20251210_integration_name_unique.ts b/api/migrations/20251210_integration_name_unique.ts new file mode 100644 index 0000000..d8f937a --- /dev/null +++ b/api/migrations/20251210_integration_name_unique.ts @@ -0,0 +1,34 @@ +import { Kysely } from "kysely"; + +export const up = async (db: Kysely): Promise => { + // Drop old constraint (org + provider + name) + await db.schema + .alterTable("integration") + .dropConstraint("integration_org_provider_name_unique") + .execute(); + + // Add new constraint (org + name only) - names must be unique per organization + await db.schema + .alterTable("integration") + .addUniqueConstraint("integration_org_name_unique", [ + "organizationId", + "name", + ]) + .execute(); +}; + +export const down = async (db: Kysely): Promise => { + await db.schema + .alterTable("integration") + .dropConstraint("integration_org_name_unique") + .execute(); + + await db.schema + .alterTable("integration") + .addUniqueConstraint("integration_org_provider_name_unique", [ + "organizationId", + "provider", + "name", + ]) + .execute(); +}; diff --git a/api/models/integration.ts b/api/models/integration.ts index 8c18594..98caefb 100644 --- a/api/models/integration.ts +++ b/api/models/integration.ts @@ -1,6 +1,8 @@ import { nanoid } from "nanoid"; +import { sql } from "kysely"; import { db } from "../db/index.ts"; import type { IntegrationTable } from "../db/schema.ts"; +import { validateIntegrationName } from "../validation/integration-name"; import type { AuthStrategy, IntegrationProvider, @@ -388,6 +390,11 @@ export const createIntegration = async ( throw new Error(`Invalid provider: ${params.provider}`); } + const nameValidation = validateIntegrationName(params.name); + if (!nameValidation.valid) { + throw new Error(nameValidation.error); + } + validateIntegrationConfig(params.authConfig); const id = nanoid(); const now = new Date().toISOString(); @@ -493,6 +500,10 @@ export const updateIntegration = async ( }; if (params.name !== undefined) { + const nameValidation = validateIntegrationName(params.name); + if (!nameValidation.valid) { + throw new Error(nameValidation.error); + } updateValues.name = params.name; } @@ -542,6 +553,11 @@ export type CreateDynamicIntegrationRequest = { export const createDynamicIntegration = async ( params: CreateDynamicIntegrationRequest ): Promise => { + const nameValidation = validateIntegrationName(params.name); + if (!nameValidation.valid) { + throw new Error(nameValidation.error); + } + validateMcpConfig(params.authConfig); const id = nanoid(); @@ -634,7 +650,7 @@ export const getIntegrationByName = async ( const row = await db .selectFrom("integration") .selectAll() - .where("name", "=", name) + .where(sql`LOWER(name)`, "=", name.toLowerCase()) .where("organizationId", "=", organizationId) .where("enabled", "=", true) .executeTakeFirst(); diff --git a/api/validation/integration-name.test.ts b/api/validation/integration-name.test.ts new file mode 100644 index 0000000..5832d39 --- /dev/null +++ b/api/validation/integration-name.test.ts @@ -0,0 +1,96 @@ +import { expect } from "@std/expect"; +import { describe, it } from "@std/testing/bdd"; +import { validateIntegrationName, INTEGRATION_NAME_PATTERN } from "./integration-name"; + +describe("Integration Name Validation", () => { + describe("INTEGRATION_NAME_PATTERN", () => { + it("matches valid lowercase names", () => { + expect(INTEGRATION_NAME_PATTERN.test("github")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("myapi")).toBe(true); + }); + + it("matches names with hyphens", () => { + expect(INTEGRATION_NAME_PATTERN.test("my-api")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("my-github-api")).toBe(true); + }); + + it("matches names with underscores", () => { + expect(INTEGRATION_NAME_PATTERN.test("my_api")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("my_github_api")).toBe(true); + }); + + it("matches names with numbers", () => { + expect(INTEGRATION_NAME_PATTERN.test("api123")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("api-v2")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("test123api")).toBe(true); + }); + + it("matches combined valid characters", () => { + expect(INTEGRATION_NAME_PATTERN.test("my_github-api_v2")).toBe(true); + expect(INTEGRATION_NAME_PATTERN.test("test-api_123")).toBe(true); + }); + + it("rejects uppercase letters", () => { + expect(INTEGRATION_NAME_PATTERN.test("GitHub")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("MyAPI")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("myAPI")).toBe(false); + }); + + it("rejects spaces", () => { + expect(INTEGRATION_NAME_PATTERN.test("my api")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("my github api")).toBe(false); + }); + + it("rejects special characters", () => { + expect(INTEGRATION_NAME_PATTERN.test("my@api")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("my.api")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("my!api")).toBe(false); + expect(INTEGRATION_NAME_PATTERN.test("my#api")).toBe(false); + }); + }); + + describe("validateIntegrationName", () => { + it("accepts valid lowercase names", () => { + expect(validateIntegrationName("github").valid).toBe(true); + expect(validateIntegrationName("my-api").valid).toBe(true); + expect(validateIntegrationName("api_v2").valid).toBe(true); + expect(validateIntegrationName("test-api-123").valid).toBe(true); + }); + + it("rejects empty names", () => { + const result = validateIntegrationName(""); + expect(result.valid).toBe(false); + expect(result.error).toContain("required"); + }); + + it("rejects whitespace-only names", () => { + const result = validateIntegrationName(" "); + expect(result.valid).toBe(false); + expect(result.error).toContain("required"); + }); + + it("rejects names with uppercase letters", () => { + const result = validateIntegrationName("GitHub"); + expect(result.valid).toBe(false); + expect(result.error).toContain("lowercase"); + }); + + it("rejects names with spaces", () => { + const result = validateIntegrationName("my api"); + expect(result.valid).toBe(false); + expect(result.error).toContain("lowercase"); + }); + + it("rejects names with special characters", () => { + const result = validateIntegrationName("my@api"); + expect(result.valid).toBe(false); + expect(result.error).toContain("lowercase"); + }); + + it("returns undefined error when valid", () => { + const result = validateIntegrationName("valid-name"); + expect(result.valid).toBe(true); + expect(result.error).toBeUndefined(); + }); + }); +}); diff --git a/api/validation/integration-name.ts b/api/validation/integration-name.ts new file mode 100644 index 0000000..e0e249e --- /dev/null +++ b/api/validation/integration-name.ts @@ -0,0 +1,22 @@ +// Pattern: lowercase letters, numbers, hyphens, underscores only +export const INTEGRATION_NAME_PATTERN = /^[a-z0-9_-]+$/; + +export interface IntegrationNameValidationResult { + valid: boolean; + error?: string; +} + +export const validateIntegrationName = (name: string): IntegrationNameValidationResult => { + if (!name || !name.trim()) { + return { valid: false, error: "Integration name is required" }; + } + + if (!INTEGRATION_NAME_PATTERN.test(name)) { + return { + valid: false, + error: "Integration name can only contain lowercase letters (a-z), numbers (0-9), hyphens (-), and underscores (_)", + }; + } + + return { valid: true }; +}; diff --git a/tests/tests/integrations-graphql.test.ts b/tests/tests/integrations-graphql.test.ts index 6ba861c..d9f0680 100644 --- a/tests/tests/integrations-graphql.test.ts +++ b/tests/tests/integrations-graphql.test.ts @@ -126,14 +126,14 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa // 1. Create integration const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "github", - name: "Test GitHub Integration", + name: "test-github-integration", authConfig: JSON.stringify(authConfig), }); expect(createResult.createIntegration).toBeDefined(); expect(createResult.createIntegration.id).toBeDefined(); expect(createResult.createIntegration.provider).toBe("github"); - expect(createResult.createIntegration.name).toBe("Test GitHub Integration"); + expect(createResult.createIntegration.name).toBe("test-github-integration"); expect(createResult.createIntegration.enabled).toBe(true); const integrationId = createResult.createIntegration.id; @@ -162,13 +162,13 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa // 4. Update integration const updateResult: any = await graphqlClient.request(UPDATE_INTEGRATION, { id: integrationId, - name: "Updated GitHub Integration", + name: "updated-github-integration", enabled: false, }); expect(updateResult.updateIntegration).toBeDefined(); expect(updateResult.updateIntegration.id).toBe(integrationId); - expect(updateResult.updateIntegration.name).toBe("Updated GitHub Integration"); + expect(updateResult.updateIntegration.name).toBe("updated-github-integration"); expect(updateResult.updateIntegration.enabled).toBe(false); // 5. Delete integration @@ -255,7 +255,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient1.request(CREATE_INTEGRATION, { provider: "github", - name: "Org 1 Integration", + name: "org-1-integration", authConfig: JSON.stringify(authConfig), }); @@ -321,7 +321,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "invalid-provider", - name: "Test Integration", + name: "test-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -365,14 +365,14 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Viewer-Scoped PAT MCP Integration", + name: "viewer-scoped-pat-mcp-integration", authConfig: JSON.stringify(authConfig), }); expect(createResult.createIntegration).toBeDefined(); expect(createResult.createIntegration.id).toBeDefined(); expect(createResult.createIntegration.provider).toBe("mcp"); - expect(createResult.createIntegration.name).toBe("Viewer-Scoped PAT MCP Integration"); + expect(createResult.createIntegration.name).toBe("viewer-scoped-pat-mcp-integration"); expect(createResult.createIntegration.enabled).toBe(true); // Verify authConfig is returned correctly @@ -420,7 +420,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Org-Level API Key MCP Integration", + name: "org-level-api-key-mcp-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -462,14 +462,14 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Public GraphQL API", + name: "public-graphql-api", authConfig: JSON.stringify(authConfig), }); expect(createResult.createIntegration).toBeDefined(); expect(createResult.createIntegration.id).toBeDefined(); expect(createResult.createIntegration.provider).toBe("graphql"); - expect(createResult.createIntegration.name).toBe("Public GraphQL API"); + expect(createResult.createIntegration.name).toBe("public-graphql-api"); expect(createResult.createIntegration.enabled).toBe(true); const returnedAuthConfig = JSON.parse(createResult.createIntegration.authConfig); @@ -511,7 +511,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "GraphQL with Org API Key", + name: "graphql-with-org-api-key", authConfig: JSON.stringify(authConfig), }); @@ -560,7 +560,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "GraphQL with Viewer PAT", + name: "graphql-with-viewer-pat", authConfig: JSON.stringify(authConfig), }); @@ -611,7 +611,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "GraphQL with OAuth", + name: "graphql-with-oauth", authConfig: JSON.stringify(authConfig), }); @@ -662,7 +662,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa const createResult: any = await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "GraphQL with Custom Headers", + name: "graphql-with-custom-headers", authConfig: JSON.stringify(authConfig), }); @@ -703,7 +703,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Invalid GraphQL Integration", + name: "invalid-graphql-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -740,7 +740,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Invalid GraphQL Integration", + name: "invalid-graphql-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -783,7 +783,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Invalid GraphQL Integration", + name: "invalid-graphql-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -825,7 +825,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Invalid GraphQL Integration", + name: "invalid-graphql-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); @@ -869,7 +869,7 @@ describe("Integrations GraphQL API", { sanitizeOps: false, sanitizeResources: fa try { await graphqlClient.request(CREATE_INTEGRATION, { provider: "graphql", - name: "Invalid GraphQL Integration", + name: "invalid-graphql-integration", authConfig: JSON.stringify(authConfig), }); throw new Error("Should have thrown error"); diff --git a/tests/tests/pat-link.test.ts b/tests/tests/pat-link.test.ts index 9b9ec57..c1ccdee 100644 --- a/tests/tests/pat-link.test.ts +++ b/tests/tests/pat-link.test.ts @@ -102,7 +102,7 @@ describe("PAT Link API", { sanitizeOps: false, sanitizeResources: false }, () => const createResult = (await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Test PAT Integration", + name: "test-pat-integration", authConfig: JSON.stringify(authConfig), })) as { createIntegration: { id: string; name: string } }; @@ -125,7 +125,7 @@ describe("PAT Link API", { sanitizeOps: false, sanitizeResources: false }, () => expect(response.status).toBe(200); expect(data.id).toBe(patLinkId); expect(data.integration).toBeDefined(); - expect(data.integration.name).toBe("Test PAT Integration"); + expect(data.integration.name).toBe("test-pat-integration"); expect(data.integration.authInstructions).toBe( "Please enter your API key from Settings > API Keys" ); @@ -184,7 +184,7 @@ describe("PAT Link API", { sanitizeOps: false, sanitizeResources: false }, () => const createResult = (await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Test PAT Submit Integration", + name: "test-pat-submit-integration", authConfig: JSON.stringify(authConfig), })) as { createIntegration: { id: string; name: string } }; @@ -251,7 +251,7 @@ describe("PAT Link API", { sanitizeOps: false, sanitizeResources: false }, () => const createResult = (await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Test PAT Used Integration", + name: "test-pat-used-integration", authConfig: JSON.stringify(authConfig), })) as { createIntegration: { id: string; name: string } }; @@ -342,7 +342,7 @@ describe("PAT Link API", { sanitizeOps: false, sanitizeResources: false }, () => const createResult = (await graphqlClient.request(CREATE_INTEGRATION, { provider: "mcp", - name: "Test PAT Required Integration", + name: "test-pat-required-integration", authConfig: JSON.stringify(authConfig), })) as { createIntegration: { id: string; name: string } }; diff --git a/tests/tests/subroutine-integrations.test.ts b/tests/tests/subroutine-integrations.test.ts index 7e11444..55d673c 100644 --- a/tests/tests/subroutine-integrations.test.ts +++ b/tests/tests/subroutine-integrations.test.ts @@ -104,7 +104,7 @@ describe("Subroutine integrations", { sanitizeOps: false, sanitizeResources: fal const integrationResult = await graphqlClient.request(CREATE_INTEGRATION, { provider: "gmail", - name: `Gmail Integration ${crypto.randomUUID()}`, + name: `gmail-integration-${crypto.randomUUID()}`, authConfig: JSON.stringify(authConfig), }); @@ -172,7 +172,7 @@ describe("Subroutine integrations", { sanitizeOps: false, sanitizeResources: fal const integrationResult = await graphqlClient.request(CREATE_INTEGRATION, { provider: "mock_oauth", - name: `Mock OAuth Integration ${crypto.randomUUID()}`, + name: `mock-oauth-integration-${crypto.randomUUID()}`, authConfig: JSON.stringify(authConfig), });