From 2b9b45ea761732868f1a117b1ce38ba249003adf Mon Sep 17 00:00:00 2001 From: Nivedin <53208152+nivedin@users.noreply.github.com> Date: Tue, 7 Oct 2025 17:15:06 +0530 Subject: [PATCH] fix: prevent syncing secret variable initial values (#5434) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: jamesgeorge007 <25279263+jamesgeorge007@users.noreply.github.com> --- .../components/environments/my/Details.vue | 22 +++++- .../components/environments/teams/Details.vue | 22 +++++- .../src/helpers/RequestRunner.ts | 77 ++++++++++++------- .../editor/extensions/HoppEnvironment.ts | 13 +++- .../src/helpers/utils/environments.ts | 2 + .../src/newstore/environments.ts | 44 +++++++++-- .../secret-environment.service.spec.ts | 33 ++++++-- .../persistence/validation-schemas/index.ts | 1 + .../services/secret-environment.service.ts | 27 ++++++- 9 files changed, 187 insertions(+), 54 deletions(-) diff --git a/packages/hoppscotch-common/src/components/environments/my/Details.vue b/packages/hoppscotch-common/src/components/environments/my/Details.vue index 6ceb12bd..ba7e68e4 100644 --- a/packages/hoppscotch-common/src/components/environments/my/Details.vue +++ b/packages/hoppscotch-common/src/components/environments/my/Details.vue @@ -436,7 +436,7 @@ const workingEnvID = computed(() => { const getCurrentValue = (id: string | "Global", varIndex: number) => { const env = workingEnv.value?.variables[varIndex] - if (env && env.secret) { + if (env?.secret) { return secretEnvironmentService.getSecretEnvironmentVariable(id, varIndex) ?.value } @@ -444,6 +444,15 @@ const getCurrentValue = (id: string | "Global", varIndex: number) => { ?.currentValue } +const getInitialValue = (id: string | "Global", varIndex: number) => { + const env = workingEnv.value?.variables[varIndex] + if (env?.secret) { + return secretEnvironmentService.getSecretEnvironmentVariable(id, varIndex) + ?.initialValue + } + return env?.initialValue +} + watch( () => props.show, (show) => { @@ -469,7 +478,13 @@ watch( : workingEnvID.value, index ) ?? e.currentValue, - initialValue: e.initialValue, + initialValue: + getInitialValue( + props.editingEnvironmentIndex === "Global" + ? "Global" + : workingEnvID.value, + index + ) ?? e.initialValue, secret: e.secret, }, })) @@ -535,6 +550,7 @@ const saveEnvironment = () => { key: e.key, value: e.currentValue, varIndex: i, + initialValue: e.initialValue, }) : O.none ) @@ -583,7 +599,7 @@ const saveEnvironment = () => { A.map((e) => ({ key: e.key, secret: e.secret, - initialValue: e.initialValue || "", + initialValue: e.secret ? "" : e.initialValue, currentValue: "", })) ) diff --git a/packages/hoppscotch-common/src/components/environments/teams/Details.vue b/packages/hoppscotch-common/src/components/environments/teams/Details.vue index ba40ed11..18814e70 100644 --- a/packages/hoppscotch-common/src/components/environments/teams/Details.vue +++ b/packages/hoppscotch-common/src/components/environments/teams/Details.vue @@ -419,6 +419,13 @@ const getCurrentValue = ( )?.currentValue } +const getInitialValue = (editingID: string, varIndex: number) => { + return secretEnvironmentService.getSecretEnvironmentVariable( + editingID, + varIndex + )?.initialValue +} + watch( () => props.show, (show) => { @@ -449,7 +456,11 @@ watch( index, e.secret ) ?? e.currentValue, - initialValue: e.initialValue, + initialValue: e.secret + ? (getInitialValue(props.editingEnvironment?.id ?? "", index) ?? + e.initialValue ?? + "") + : e.initialValue, secret: e.secret, }, })) @@ -516,7 +527,12 @@ const saveEnvironment = async () => { filteredVariables, A.filterMapWithIndex((i, e) => e.secret - ? O.some({ key: e.key, value: e.currentValue, varIndex: i }) + ? O.some({ + key: e.key, + value: e.currentValue, + varIndex: i, + initialValue: e.initialValue, + }) : O.none ) ) @@ -540,7 +556,7 @@ const saveEnvironment = async () => { A.map((e) => ({ key: e.key, secret: e.secret, - initialValue: e.initialValue || "", + initialValue: e.secret ? "" : e.initialValue, currentValue: "", })) ) diff --git a/packages/hoppscotch-common/src/helpers/RequestRunner.ts b/packages/hoppscotch-common/src/helpers/RequestRunner.ts index 9c06f737..7824af60 100644 --- a/packages/hoppscotch-common/src/helpers/RequestRunner.ts +++ b/packages/hoppscotch-common/src/helpers/RequestRunner.ts @@ -167,15 +167,15 @@ const updateEnvironments = ( key: e.key, value: e.currentValue ?? "", varIndex: index, + initialValue: e.initialValue ?? "", }) - // delete the value from the environment - // so that it doesn't get saved in the environment - + // create a new object with cleared values for secret variables + // so that these values don't get saved in the environment return { key: e.key, secret: e.secret, - initialValue: e.initialValue ?? "", + initialValue: e.secret ? "" : (e.initialValue ?? ""), currentValue: "", } } @@ -210,24 +210,36 @@ const updateEnvironments = ( return updatedEnv } +/** + * Get the environment variable value from the secret environment service + * @param envID The environment ID + * @param index The index of the environment variable + * @returns Current value and initial value of the environment variable + */ +const getSecretEnvironmentVariableValue = ( + envID: string, + index: number +): { + value: string + initialValue?: string +} | null => { + return secretEnvironmentService.getSecretEnvironmentVariableValue( + envID, + index + ) +} + /** * Get the environment variable value from the current environment * @param envID The environment ID * @param index The index of the environment variable * @param isSecret Whether the environment variable is a secret - * @returns The environment variable value + * @returns Current value of the environment variable */ const getEnvironmentVariableValue = ( envID: string, - index: number, - isSecret: boolean + index: number ): string | undefined => { - if (isSecret) { - return secretEnvironmentService.getSecretEnvironmentVariableValue( - envID, - index - ) - } return currentEnvironmentValueService.getEnvironmentVariableValue( envID, index @@ -823,6 +835,27 @@ const getUpdatedEnvVariables = ( ) ) +// Helper to resolve currentValue & initialValue for (secret/non-secret) env vars +const resolveEnvVars = ( + envID: string, + vars: Environment["variables"] +): Environment["variables"] => + vars.map((v, index) => { + const secretMeta = v.secret + ? getSecretEnvironmentVariableValue(envID, index) + : null + return { + ...v, + currentValue: + (v.secret + ? secretMeta?.value + : getEnvironmentVariableValue(envID, index)) ?? "", + // fallback to var initialValue if secretMeta is not found + initialValue: + (v.secret ? secretMeta?.initialValue : "") ?? v.initialValue, + } + }) + function translateToSandboxTestResults( testDesc: SandboxTestResult ): HoppTestResult { @@ -834,20 +867,10 @@ function translateToSandboxTestResults( } } - const globals = cloneDeep(getGlobalVariables()).map((g, index) => ({ - ...g, - currentValue: getEnvironmentVariableValue("Global", index, g.secret) ?? "", - })) - - const envVars = getCurrentEnvironment().variables.map((e, index) => ({ - ...e, - currentValue: - getEnvironmentVariableValue( - getCurrentEnvironment().id, - index, - e.secret - ) ?? "", - })) + const globals = resolveEnvVars("Global", cloneDeep(getGlobalVariables())) + const { id: currentEnvID, variables: currentEnvVariables } = + getCurrentEnvironment() + const envVars = resolveEnvVars(currentEnvID, currentEnvVariables) return { description: "", diff --git a/packages/hoppscotch-common/src/helpers/editor/extensions/HoppEnvironment.ts b/packages/hoppscotch-common/src/helpers/editor/extensions/HoppEnvironment.ts index a7442326..f0c3fcaa 100644 --- a/packages/hoppscotch-common/src/helpers/editor/extensions/HoppEnvironment.ts +++ b/packages/hoppscotch-common/src/helpers/editor/extensions/HoppEnvironment.ts @@ -145,10 +145,15 @@ const cursorTooltipField = (aggregateEnvs: AggregateEnvironment[]) => ? tooltipEnv.sourceEnvID! : currentSelectedEnvironment.id - const hasSecretStored = secretEnvironmentService.hasSecretValue( + const hasSecretValueStored = secretEnvironmentService.hasSecretValue( tooltipSourceEnvID, tooltipEnv?.key ?? "" ) + const hasSecretInitialValueStored = + secretEnvironmentService.hasSecretInitialValue( + tooltipSourceEnvID, + tooltipEnv?.key ?? "" + ) // We need to check if the environment is a secret and if it has a secret value stored in the secret environment service // If it is a secret and has a secret value, we need to show "******" in the tooltip @@ -158,12 +163,12 @@ const cursorTooltipField = (aggregateEnvs: AggregateEnvironment[]) => // If the source environment is not found, we need to show "Not Found" in the tooltip, ie the the environment // is not defined in the selected environment or the global environment if (isSecret) { - if (!hasSecretStored && envInitialValue) { + if (hasSecretValueStored && hasSecretInitialValueStored) { envInitialValue = "******" - } else if (hasSecretStored && !envInitialValue) { envCurrentValue = "******" - } else if (hasSecretStored && envInitialValue) { + } else if (!hasSecretValueStored && hasSecretInitialValueStored) { envInitialValue = "******" + } else if (hasSecretValueStored && !hasSecretInitialValueStored) { envCurrentValue = "******" } else { envInitialValue = "Empty" diff --git a/packages/hoppscotch-common/src/helpers/utils/environments.ts b/packages/hoppscotch-common/src/helpers/utils/environments.ts index d197c384..c466e2a3 100644 --- a/packages/hoppscotch-common/src/helpers/utils/environments.ts +++ b/packages/hoppscotch-common/src/helpers/utils/environments.ts @@ -36,6 +36,7 @@ const unWrapEnvironments = ( return { ...globalVar, currentValue: secretVar.value, + initialValue: secretVar.initialValue ?? "", } } return { @@ -58,6 +59,7 @@ const unWrapEnvironments = ( return { ...selectedVar, currentValue: secretVar.value, + initialValue: secretVar.initialValue ?? "", } } return { diff --git a/packages/hoppscotch-common/src/newstore/environments.ts b/packages/hoppscotch-common/src/newstore/environments.ts index 6013dba7..de313c89 100644 --- a/packages/hoppscotch-common/src/newstore/environments.ts +++ b/packages/hoppscotch-common/src/newstore/environments.ts @@ -556,12 +556,19 @@ export function getAggregateEnvsWithCurrentValue() { ...currentEnv.variables.map((x, index) => { let currentValue = x.currentValue + let initialValue = x.initialValue if (x.secret) { currentValue = secretEnvironmentService.getSecretEnvironmentVariableValue( currentEnv.id, index - ) ?? "" + )?.value ?? "" + + initialValue = + secretEnvironmentService.getSecretEnvironmentVariableValue( + currentEnv.id, + index + )?.initialValue ?? "" } return { @@ -571,19 +578,26 @@ export function getAggregateEnvsWithCurrentValue() { currentEnv.id, index ) ?? currentValue, - initialValue: x.initialValue, + initialValue: x.initialValue ?? initialValue, secret: x.secret, sourceEnv: currentEnv.name, } }), ...getGlobalVariables().map((x, index) => { let currentValue = x.currentValue + let initialValue = x.initialValue if (x.secret) { currentValue = secretEnvironmentService.getSecretEnvironmentVariableValue( "Global", index - ) ?? "" + )?.value ?? "" + + initialValue = + secretEnvironmentService.getSecretEnvironmentVariableValue( + "Global", + index + )?.initialValue ?? "" } return { key: x.key, @@ -592,7 +606,7 @@ export function getAggregateEnvsWithCurrentValue() { "Global", index ) ?? currentValue, - initialValue: x.initialValue, + initialValue: x.initialValue ?? initialValue, secret: x.secret, sourceEnv: "Global", } @@ -623,12 +637,19 @@ export const aggregateEnvsWithCurrentValue$: Observable< selectedEnv?.variables.map((x, index) => { let currentValue = x.currentValue + let initialValue = x.initialValue if (x.secret) { currentValue = secretEnvironmentService.getSecretEnvironmentVariableValue( selectedEnv.id, index - ) ?? "" + )?.value ?? "" + + initialValue = + secretEnvironmentService.getSecretEnvironmentVariableValue( + selectedEnv.id, + index + )?.initialValue ?? "" } results.push({ key: x.key, @@ -637,7 +658,7 @@ export const aggregateEnvsWithCurrentValue$: Observable< selectedEnv.id, index ) ?? currentValue, - initialValue: x.initialValue, + initialValue: x.initialValue ?? initialValue, secret: x.secret, sourceEnv: selectedEnv.name, }) @@ -645,12 +666,19 @@ export const aggregateEnvsWithCurrentValue$: Observable< globalEnv.variables.map((x, index) => { let currentValue = x.currentValue + let initialValue = x.initialValue if (x.secret) { currentValue = secretEnvironmentService.getSecretEnvironmentVariableValue( "Global", index - ) ?? "" + )?.value ?? "" + + initialValue = + secretEnvironmentService.getSecretEnvironmentVariableValue( + "Global", + index + )?.initialValue ?? "" } results.push({ key: x.key, @@ -659,7 +687,7 @@ export const aggregateEnvsWithCurrentValue$: Observable< "Global", index ) ?? currentValue, - initialValue: x.initialValue, + initialValue: x.initialValue ?? initialValue, secret: x.secret, sourceEnv: "Global", }) diff --git a/packages/hoppscotch-common/src/services/__tests__/secret-environment.service.spec.ts b/packages/hoppscotch-common/src/services/__tests__/secret-environment.service.spec.ts index a3958957..a4390ec6 100644 --- a/packages/hoppscotch-common/src/services/__tests__/secret-environment.service.spec.ts +++ b/packages/hoppscotch-common/src/services/__tests__/secret-environment.service.spec.ts @@ -72,18 +72,37 @@ describe("SecretEnvironmentService", () => { }) describe("getSecretEnvironmentVariableValue", () => { - it("should return the value of the specified secret environment variable", () => { + it("should return the value and initialValue of the specified secret environment variable", () => { const id = "testEnvironment" - const secretVars = [{ key: "key1", value: "value1", varIndex: 1 }] + const secretVars = [ + { key: "key1", value: "value1", initialValue: "init1", varIndex: 1 }, + ] service.secretEnvironments.set(id, secretVars) const result = service.getSecretEnvironmentVariableValue(id, 1) - expect(result).toEqual(secretVars[0].value) + expect(result).toEqual({ + value: "value1", + initialValue: "init1", + }) }) - it("should return undefined if the specified variable does not exist", () => { + it("should return null if the variable has no value/initialValue", () => { + const id = "testEnvironment" + const secretVars = [{ key: "key1", varIndex: 1 }] + + service.secretEnvironments.set(id, secretVars as any) + + const result = service.getSecretEnvironmentVariableValue(id, 1) + + expect(result).toEqual({ + value: "", + initialValue: "", + }) + }) + + it("should return null if the specified variable does not exist", () => { const id = "testEnvironment" const secretVars = [{ key: "key1", value: "value1", varIndex: 1 }] @@ -91,15 +110,15 @@ describe("SecretEnvironmentService", () => { const result = service.getSecretEnvironmentVariableValue(id, 2) - expect(result).toBeUndefined() + expect(result).toBeNull() }) - it("should return undefined if the specified environment does not exist", () => { + it("should return null if the specified environment does not exist", () => { const id = "nonExistentEnvironment" const result = service.getSecretEnvironmentVariableValue(id, 1) - expect(result).toBeUndefined() + expect(result).toBeNull() }) }) diff --git a/packages/hoppscotch-common/src/services/persistence/validation-schemas/index.ts b/packages/hoppscotch-common/src/services/persistence/validation-schemas/index.ts index d8a5d9b3..10e94aaf 100644 --- a/packages/hoppscotch-common/src/services/persistence/validation-schemas/index.ts +++ b/packages/hoppscotch-common/src/services/persistence/validation-schemas/index.ts @@ -379,6 +379,7 @@ export const SECRET_ENVIRONMENT_VARIABLE_SCHEMA = z.union([ .object({ key: z.string(), value: z.string(), + initialValue: z.string().optional().catch(""), varIndex: z.number(), }) .strict() diff --git a/packages/hoppscotch-common/src/services/secret-environment.service.ts b/packages/hoppscotch-common/src/services/secret-environment.service.ts index 5b16c8d7..f42841be 100644 --- a/packages/hoppscotch-common/src/services/secret-environment.service.ts +++ b/packages/hoppscotch-common/src/services/secret-environment.service.ts @@ -4,11 +4,15 @@ import { reactive, computed, watch } from "vue" /** * Defines a secret environment variable. + * Value is the current value of the variable. + * InitialValue is the value of the variable when it was created. + * VarIndex is the index of the variable in the environment. */ export type SecretVariable = { key: string value: string varIndex: number + initialValue?: string } /** @@ -61,13 +65,17 @@ export class SecretEnvironmentService extends Service { } /** - * Used to get the value of a secret environment variable. + * Used to get the initial and current value of a secret environment variable. * @param id ID of the environment * @param varIndex Index of the variable in the environment */ public getSecretEnvironmentVariableValue(id: string, varIndex: number) { const secretVar = this.getSecretEnvironmentVariable(id, varIndex) - return secretVar?.value + if (!secretVar) return null + return { + value: secretVar.value || "", + initialValue: secretVar.initialValue || "", + } } /** @@ -134,6 +142,21 @@ export class SecretEnvironmentService extends Service { ) } + /** + * Checks if a secret variable has an initial value set. + * @param id ID of the environment + * @param key Key of the variable to check the initial value exists + * @returns true if the key has an initial value + */ + public hasSecretInitialValue(id: string, key: string) { + return ( + this.secretEnvironments.has(id) && + this.secretEnvironments + .get(id)! + .some((secretVar) => secretVar.key === key && secretVar.initialValue) + ) + } + /** * Used to update the value of a secret environment variable. */