From 76a424dcfb14c790be25c029116826de4933b5d1 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Fri, 7 Jun 2024 19:00:51 +0800 Subject: [PATCH 1/4] feat: added option for disabling github secret deletion --- .../integration-sync-secret.ts | 65 ++++++++++--------- .../src/pages/integrations/github/create.tsx | 31 +++++++-- 2 files changed, 59 insertions(+), 37 deletions(-) diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 9d8ee8895..123578e70 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -1363,38 +1363,41 @@ const syncSecretsGitHub = async ({ } } - for await (const encryptedSecret of encryptedSecrets) { - if ( - !(encryptedSecret.name in secrets) && - !(appendices?.prefix !== undefined && !encryptedSecret.name.startsWith(appendices?.prefix)) && - !(appendices?.suffix !== undefined && !encryptedSecret.name.endsWith(appendices?.suffix)) - ) { - switch (integration.scope) { - case GithubScope.Org: { - await octokit.request("DELETE /orgs/{org}/actions/secrets/{secret_name}", { - org: integration.owner as string, - secret_name: encryptedSecret.name - }); - break; - } - case GithubScope.Env: { - await octokit.request( - "DELETE /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}", - { - repository_id: Number(integration.appId), - environment_name: integration.targetEnvironmentId as string, + const metadata = z.record(z.any()).parse(integration.metadata); + if (!metadata.shouldDisableDelete) { + for await (const encryptedSecret of encryptedSecrets) { + if ( + !(encryptedSecret.name in secrets) && + !(appendices?.prefix !== undefined && !encryptedSecret.name.startsWith(appendices?.prefix)) && + !(appendices?.suffix !== undefined && !encryptedSecret.name.endsWith(appendices?.suffix)) + ) { + switch (integration.scope) { + case GithubScope.Org: { + await octokit.request("DELETE /orgs/{org}/actions/secrets/{secret_name}", { + org: integration.owner as string, secret_name: encryptedSecret.name - } - ); - break; - } - default: { - await octokit.request("DELETE /repos/{owner}/{repo}/actions/secrets/{secret_name}", { - owner: integration.owner as string, - repo: integration.app as string, - secret_name: encryptedSecret.name - }); - break; + }); + break; + } + case GithubScope.Env: { + await octokit.request( + "DELETE /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}", + { + repository_id: Number(integration.appId), + environment_name: integration.targetEnvironmentId as string, + secret_name: encryptedSecret.name + } + ); + break; + } + default: { + await octokit.request("DELETE /repos/{owner}/{repo}/actions/secrets/{secret_name}", { + owner: integration.owner as string, + repo: integration.app as string, + secret_name: encryptedSecret.name + }); + break; + } } } } diff --git a/frontend/src/pages/integrations/github/create.tsx b/frontend/src/pages/integrations/github/create.tsx index e42a7e9eb..2a92b07e6 100644 --- a/frontend/src/pages/integrations/github/create.tsx +++ b/frontend/src/pages/integrations/github/create.tsx @@ -33,6 +33,7 @@ import { Input, Select, SelectItem, + Switch, Tab, TabList, TabPanel, @@ -59,7 +60,7 @@ const schema = yup.object({ selectedSourceEnvironment: yup.string().trim().required("Project Environment is required"), secretPath: yup.string().trim().required("Secrets Path is required"), secretSuffix: yup.string().trim().optional(), - + shouldDisableDelete: yup.boolean().optional(), scope: yup.mixed().oneOf(targetEnv.slice()).required(), repoIds: yup.mixed().when("scope", { @@ -98,7 +99,6 @@ type FormData = yup.InferType; export default function GitHubCreateIntegrationPage() { const router = useRouter(); const { mutateAsync } = useCreateIntegration(); - const integrationAuthId = (queryString.parse(router.asPath.split("?")[1]).integrationAuthId as string) ?? ""; @@ -120,7 +120,8 @@ export default function GitHubCreateIntegrationPage() { defaultValues: { secretPath: "/", scope: "github-repo", - repoIds: [] + repoIds: [], + shouldDisableDelete: false } }); @@ -177,7 +178,8 @@ export default function GitHubCreateIntegrationPage() { app: targetApp.name, // repo name owner: targetApp.owner, // repo owner metadata: { - secretSuffix: data.secretSuffix + secretSuffix: data.secretSuffix, + shouldDisableDelete: data.shouldDisableDelete } }); }) @@ -194,7 +196,8 @@ export default function GitHubCreateIntegrationPage() { scope: data.scope, owner: integrationAuthOrgs?.find((e) => e.orgId === data.orgId)?.name, metadata: { - secretSuffix: data.secretSuffix + secretSuffix: data.secretSuffix, + shouldDisableDelete: data.shouldDisableDelete } }); break; @@ -211,7 +214,8 @@ export default function GitHubCreateIntegrationPage() { owner: repoOwner, targetEnvironmentId: data.envId, metadata: { - secretSuffix: data.secretSuffix + secretSuffix: data.secretSuffix, + shouldDisableDelete: data.shouldDisableDelete } }); break; @@ -546,6 +550,21 @@ export default function GitHubCreateIntegrationPage() { animate={{ opacity: 1, translateX: 0 }} exit={{ opacity: 0, translateX: 30 }} > +
+ ( + onChange(isChecked)} + isChecked={value} + > + Disable secrets deletion on Github + + )} + /> +
Date: Sat, 8 Jun 2024 00:30:30 +0800 Subject: [PATCH 2/4] adjustment: made secret-deletion opt in --- backend/src/lib/api-docs/constants.ts | 3 ++- backend/src/server/routes/v1/integration-router.ts | 3 ++- .../integration-auth/integration-sync-secret.ts | 2 +- .../src/services/integration/integration-types.ts | 2 ++ frontend/src/hooks/api/integrations/queries.tsx | 1 + frontend/src/pages/integrations/github/create.tsx | 14 +++++++------- 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/backend/src/lib/api-docs/constants.ts b/backend/src/lib/api-docs/constants.ts index da82016f1..18db346dc 100644 --- a/backend/src/lib/api-docs/constants.ts +++ b/backend/src/lib/api-docs/constants.ts @@ -674,7 +674,8 @@ export const INTEGRATION = { secretGCPLabel: "The label for GCP secrets.", secretAWSTag: "The tags for AWS secrets.", kmsKeyId: "The ID of the encryption key from AWS KMS.", - shouldDisableDelete: "The flag to disable deletion of secrets in AWS Parameter Store." + shouldDisableDelete: "The flag to disable deletion of secrets in AWS Parameter Store.", + shouldEnableDelete: "The flag to enable deletion of secrets" } }, UPDATE: { diff --git a/backend/src/server/routes/v1/integration-router.ts b/backend/src/server/routes/v1/integration-router.ts index bdb58aa8b..70f59d558 100644 --- a/backend/src/server/routes/v1/integration-router.ts +++ b/backend/src/server/routes/v1/integration-router.ts @@ -73,7 +73,8 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { .optional() .describe(INTEGRATION.CREATE.metadata.secretAWSTag), kmsKeyId: z.string().optional().describe(INTEGRATION.CREATE.metadata.kmsKeyId), - shouldDisableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldDisableDelete) + shouldDisableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldDisableDelete), + shouldEnableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldEnableDelete) }) .default({}) }), diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 123578e70..16016a0c0 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -1364,7 +1364,7 @@ const syncSecretsGitHub = async ({ } const metadata = z.record(z.any()).parse(integration.metadata); - if (!metadata.shouldDisableDelete) { + if (metadata.shouldEnableDelete) { for await (const encryptedSecret of encryptedSecrets) { if ( !(encryptedSecret.name in secrets) && diff --git a/backend/src/services/integration/integration-types.ts b/backend/src/services/integration/integration-types.ts index 9c75cad2d..db136c1ed 100644 --- a/backend/src/services/integration/integration-types.ts +++ b/backend/src/services/integration/integration-types.ts @@ -29,6 +29,7 @@ export type TCreateIntegrationDTO = { }[]; kmsKeyId?: string; shouldDisableDelete?: boolean; + shouldEnableDelete?: boolean; }; } & Omit; @@ -54,6 +55,7 @@ export type TUpdateIntegrationDTO = { }[]; kmsKeyId?: string; shouldDisableDelete?: boolean; + shouldEnableDelete?: boolean; }; } & Omit; diff --git a/frontend/src/hooks/api/integrations/queries.tsx b/frontend/src/hooks/api/integrations/queries.tsx index 3aa8f3ed1..e561280c6 100644 --- a/frontend/src/hooks/api/integrations/queries.tsx +++ b/frontend/src/hooks/api/integrations/queries.tsx @@ -73,6 +73,7 @@ export const useCreateIntegration = () => { }[]; kmsKeyId?: string; shouldDisableDelete?: boolean; + shouldEnableDelete?: boolean; }; }) => { const { diff --git a/frontend/src/pages/integrations/github/create.tsx b/frontend/src/pages/integrations/github/create.tsx index 2a92b07e6..1348f2e04 100644 --- a/frontend/src/pages/integrations/github/create.tsx +++ b/frontend/src/pages/integrations/github/create.tsx @@ -60,7 +60,7 @@ const schema = yup.object({ selectedSourceEnvironment: yup.string().trim().required("Project Environment is required"), secretPath: yup.string().trim().required("Secrets Path is required"), secretSuffix: yup.string().trim().optional(), - shouldDisableDelete: yup.boolean().optional(), + shouldEnableDelete: yup.boolean().optional(), scope: yup.mixed().oneOf(targetEnv.slice()).required(), repoIds: yup.mixed().when("scope", { @@ -121,7 +121,7 @@ export default function GitHubCreateIntegrationPage() { secretPath: "/", scope: "github-repo", repoIds: [], - shouldDisableDelete: false + shouldEnableDelete: false } }); @@ -179,7 +179,7 @@ export default function GitHubCreateIntegrationPage() { owner: targetApp.owner, // repo owner metadata: { secretSuffix: data.secretSuffix, - shouldDisableDelete: data.shouldDisableDelete + shouldEnableDelete: data.shouldEnableDelete } }); }) @@ -197,7 +197,7 @@ export default function GitHubCreateIntegrationPage() { owner: integrationAuthOrgs?.find((e) => e.orgId === data.orgId)?.name, metadata: { secretSuffix: data.secretSuffix, - shouldDisableDelete: data.shouldDisableDelete + shouldEnableDelete: data.shouldEnableDelete } }); break; @@ -215,7 +215,7 @@ export default function GitHubCreateIntegrationPage() { targetEnvironmentId: data.envId, metadata: { secretSuffix: data.secretSuffix, - shouldDisableDelete: data.shouldDisableDelete + shouldEnableDelete: data.shouldEnableDelete } }); break; @@ -553,14 +553,14 @@ export default function GitHubCreateIntegrationPage() {
( onChange(isChecked)} isChecked={value} > - Disable secrets deletion on Github + Delete secrets not in Infisical )} /> From b9933d711cfeaf18bca6584ed17a332f579f84dc Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Mon, 10 Jun 2024 13:58:40 +0800 Subject: [PATCH 3/4] misc: addressed schema update --- .../server/routes/v1/integration-router.ts | 62 +------------------ .../integration-sync-secret.ts | 3 +- .../integration/integration-schema.ts | 35 +++++++++++ 3 files changed, 40 insertions(+), 60 deletions(-) create mode 100644 backend/src/services/integration/integration-schema.ts diff --git a/backend/src/server/routes/v1/integration-router.ts b/backend/src/server/routes/v1/integration-router.ts index 70f59d558..97a7f4d7a 100644 --- a/backend/src/server/routes/v1/integration-router.ts +++ b/backend/src/server/routes/v1/integration-router.ts @@ -8,7 +8,7 @@ import { writeLimit } from "@app/server/config/rateLimiter"; import { getTelemetryDistinctId } from "@app/server/lib/telemetry"; import { verifyAuth } from "@app/server/plugins/auth/verify-auth"; import { AuthMode } from "@app/services/auth/auth-type"; -import { IntegrationMappingBehavior } from "@app/services/integration-auth/integration-list"; +import { IntegrationMetadataSchema } from "@app/services/integration/integration-schema"; import { PostHogEventTypes, TIntegrationCreatedEvent } from "@app/services/telemetry/telemetry-types"; export const registerIntegrationRouter = async (server: FastifyZodProvider) => { @@ -46,37 +46,7 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { path: z.string().trim().optional().describe(INTEGRATION.CREATE.path), region: z.string().trim().optional().describe(INTEGRATION.CREATE.region), scope: z.string().trim().optional().describe(INTEGRATION.CREATE.scope), - metadata: z - .object({ - secretPrefix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretPrefix), - secretSuffix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretSuffix), - initialSyncBehavior: z.string().optional().describe(INTEGRATION.CREATE.metadata.initialSyncBehavoir), - mappingBehavior: z - .nativeEnum(IntegrationMappingBehavior) - .optional() - .describe(INTEGRATION.CREATE.metadata.mappingBehavior), - shouldAutoRedeploy: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldAutoRedeploy), - secretGCPLabel: z - .object({ - labelName: z.string(), - labelValue: z.string() - }) - .optional() - .describe(INTEGRATION.CREATE.metadata.secretGCPLabel), - secretAWSTag: z - .array( - z.object({ - key: z.string(), - value: z.string() - }) - ) - .optional() - .describe(INTEGRATION.CREATE.metadata.secretAWSTag), - kmsKeyId: z.string().optional().describe(INTEGRATION.CREATE.metadata.kmsKeyId), - shouldDisableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldDisableDelete), - shouldEnableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldEnableDelete) - }) - .default({}) + metadata: IntegrationMetadataSchema.default({}) }), response: { 200: z.object({ @@ -162,33 +132,7 @@ export const registerIntegrationRouter = async (server: FastifyZodProvider) => { targetEnvironment: z.string().trim().describe(INTEGRATION.UPDATE.targetEnvironment), owner: z.string().trim().describe(INTEGRATION.UPDATE.owner), environment: z.string().trim().describe(INTEGRATION.UPDATE.environment), - metadata: z - .object({ - secretPrefix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretPrefix), - secretSuffix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretSuffix), - initialSyncBehavior: z.string().optional().describe(INTEGRATION.CREATE.metadata.initialSyncBehavoir), - mappingBehavior: z.string().optional().describe(INTEGRATION.CREATE.metadata.mappingBehavior), - shouldAutoRedeploy: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldAutoRedeploy), - secretGCPLabel: z - .object({ - labelName: z.string(), - labelValue: z.string() - }) - .optional() - .describe(INTEGRATION.CREATE.metadata.secretGCPLabel), - secretAWSTag: z - .array( - z.object({ - key: z.string(), - value: z.string() - }) - ) - .optional() - .describe(INTEGRATION.CREATE.metadata.secretAWSTag), - kmsKeyId: z.string().optional().describe(INTEGRATION.CREATE.metadata.kmsKeyId), - shouldDisableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldDisableDelete) - }) - .optional() + metadata: IntegrationMetadataSchema.optional() }), response: { 200: z.object({ diff --git a/backend/src/services/integration-auth/integration-sync-secret.ts b/backend/src/services/integration-auth/integration-sync-secret.ts index 16016a0c0..3f8331d97 100644 --- a/backend/src/services/integration-auth/integration-sync-secret.ts +++ b/backend/src/services/integration-auth/integration-sync-secret.ts @@ -31,6 +31,7 @@ import { logger } from "@app/lib/logger"; import { TCreateManySecretsRawFn, TUpdateManySecretsRawFn } from "@app/services/secret/secret-types"; import { TIntegrationDALFactory } from "../integration/integration-dal"; +import { IntegrationMetadataSchema } from "../integration/integration-schema"; import { IntegrationInitialSyncBehavior, IntegrationMappingBehavior, @@ -1363,7 +1364,7 @@ const syncSecretsGitHub = async ({ } } - const metadata = z.record(z.any()).parse(integration.metadata); + const metadata = IntegrationMetadataSchema.parse(integration.metadata); if (metadata.shouldEnableDelete) { for await (const encryptedSecret of encryptedSecrets) { if ( diff --git a/backend/src/services/integration/integration-schema.ts b/backend/src/services/integration/integration-schema.ts new file mode 100644 index 000000000..d1b6bcc77 --- /dev/null +++ b/backend/src/services/integration/integration-schema.ts @@ -0,0 +1,35 @@ +import { z } from "zod"; + +import { INTEGRATION } from "@app/lib/api-docs"; + +import { IntegrationMappingBehavior } from "../integration-auth/integration-list"; + +export const IntegrationMetadataSchema = z.object({ + secretPrefix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretPrefix), + secretSuffix: z.string().optional().describe(INTEGRATION.CREATE.metadata.secretSuffix), + initialSyncBehavior: z.string().optional().describe(INTEGRATION.CREATE.metadata.initialSyncBehavoir), + mappingBehavior: z + .nativeEnum(IntegrationMappingBehavior) + .optional() + .describe(INTEGRATION.CREATE.metadata.mappingBehavior), + shouldAutoRedeploy: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldAutoRedeploy), + secretGCPLabel: z + .object({ + labelName: z.string(), + labelValue: z.string() + }) + .optional() + .describe(INTEGRATION.CREATE.metadata.secretGCPLabel), + secretAWSTag: z + .array( + z.object({ + key: z.string(), + value: z.string() + }) + ) + .optional() + .describe(INTEGRATION.CREATE.metadata.secretAWSTag), + kmsKeyId: z.string().optional().describe(INTEGRATION.CREATE.metadata.kmsKeyId), + shouldDisableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldDisableDelete), + shouldEnableDelete: z.boolean().optional().describe(INTEGRATION.CREATE.metadata.shouldEnableDelete) +}); From f4a1a00b5981f884588ba59b5454d5e52a364a07 Mon Sep 17 00:00:00 2001 From: Sheen Capadngan Date: Mon, 10 Jun 2024 14:00:56 +0800 Subject: [PATCH 4/4] misc: improved text of github option --- frontend/src/pages/integrations/github/create.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/integrations/github/create.tsx b/frontend/src/pages/integrations/github/create.tsx index 1348f2e04..f92a98943 100644 --- a/frontend/src/pages/integrations/github/create.tsx +++ b/frontend/src/pages/integrations/github/create.tsx @@ -560,7 +560,7 @@ export default function GitHubCreateIntegrationPage() { onCheckedChange={(isChecked) => onChange(isChecked)} isChecked={value} > - Delete secrets not in Infisical + Delete secrets in Github that are not in Infisical )} />