diff --git a/src/lib/components/variables/createVariableModal.svelte b/src/lib/components/variables/createVariableModal.svelte index 06b73ea54f..5ba5ef4c67 100644 --- a/src/lib/components/variables/createVariableModal.svelte +++ b/src/lib/components/variables/createVariableModal.svelte @@ -8,6 +8,7 @@ import { Link } from '$lib/elements'; import { page } from '$app/state'; import { IconPlus, IconX } from '@appwrite.io/pink-icons-svelte'; + import { validateVariables } from '$lib/helpers/variables'; export type ProductLabel = 'site' | 'function'; @@ -39,13 +40,13 @@ newVariables = newVariables.map((variable) => ({ ...variable, secret: true })); } - newVariables.forEach((variable) => { - if (('' + variable.value).length > 8192) { - throw new Error( - `Variable ${variable.key} is longer than 8192 allowed characters` - ); - } - }); + const validationError = validateVariables( + newVariables.filter((variable) => variable.key || variable.value) + ); + if (validationError) { + throw new Error(validationError); + } + const updatedVariables = [...variables]; newVariables.forEach((newVar) => { if (!newVar.key) { diff --git a/src/lib/components/variables/environmentVariables.svelte b/src/lib/components/variables/environmentVariables.svelte index 595cc08ac5..88dddf3019 100644 --- a/src/lib/components/variables/environmentVariables.svelte +++ b/src/lib/components/variables/environmentVariables.svelte @@ -22,6 +22,7 @@ import UpdateVariableModal from './updateVariableModal.svelte'; import { Click, trackEvent } from '$lib/actions/analytics'; import { isSmallViewport } from '$lib/stores/viewport'; + import { isValidVariableKey } from '$lib/helpers/variables'; const DOCS_LINKS: Record = { site: 'https://appwrite.io/docs/products/sites/develop#accessing-environment-variables', @@ -146,7 +147,28 @@ {#each paginatedItems as variable} - {variable.key} + + + {#if !isValidVariableKey(variable.key)} + + + + This key can't be used as an environment + variable name. Rename it using only letters, + digits and underscores, without starting + with a digit. + + + {/if} + {variable.key} + +
diff --git a/src/lib/components/variables/importVariablesModal.svelte b/src/lib/components/variables/importVariablesModal.svelte index 9f0b782d1e..9eed108b23 100644 --- a/src/lib/components/variables/importVariablesModal.svelte +++ b/src/lib/components/variables/importVariablesModal.svelte @@ -7,6 +7,7 @@ import { Icon, Layout, Selector, Tooltip, Typography, Upload } from '@appwrite.io/pink-svelte'; import { parse } from '$lib/helpers/envfile'; import { removeFile } from '$lib/helpers/files'; + import { validateVariables } from '$lib/helpers/variables'; export let show = false; export let variables: Partial[]; @@ -35,19 +36,20 @@ if (!Object.keys(uploaded).length) { throw new Error('No variables found'); } - const entries = Object.entries(uploaded); + // Drop the valueless entries first. They are never written, so an + // invalid key on one of them must not reject the whole file. + const entries = Object.entries(uploaded).filter(([, value]) => !!value); - for (const [key, value] of entries) { - if (value.length > 8192) { - throw new Error(`Variable ${key} is longer than 8192 allowed characters`); - } + const validationError = validateVariables( + entries.map(([key, value]) => ({ key, value })) + ); + if (validationError) { + throw new Error(validationError); } - entries - .filter(([, value]) => !!value) - .forEach(([key, value]) => { - variables.push({ key, value, secret }); - }); + entries.forEach(([key, value]) => { + variables.push({ key, value, secret }); + }); show = false; } catch (e) { diff --git a/src/lib/components/variables/updateVariableModal.svelte b/src/lib/components/variables/updateVariableModal.svelte index 7d02391fab..005ec89ba0 100644 --- a/src/lib/components/variables/updateVariableModal.svelte +++ b/src/lib/components/variables/updateVariableModal.svelte @@ -7,6 +7,7 @@ import { Layout, Selector } from '@appwrite.io/pink-svelte'; import { Link } from '$lib/elements'; import { page } from '$app/state'; + import { validateVariables } from '$lib/helpers/variables'; export let show = false; export let selectedVar: Partial; @@ -20,7 +21,15 @@ secret: selectedVar?.secret }; + let error = ''; + function handleVariable() { + const validationError = validateVariables([pair]); + if (validationError) { + error = validationError; + return; + } + if (selectedVar) { variables = variables.map((variable) => { const match = selectedVar.$id @@ -39,7 +48,7 @@ } - + Update the environment variable for your {productLabel}. Global variables can be set in 8192) { - throw new Error(`Variable ${key} is longer than 8192 allowed characters`); - } + const validationError = validateVariables( + entries.map(([key, value]) => ({ key, value })) + ); + if (validationError) { + throw new Error(validationError); } // Update or remove editable variables diff --git a/src/lib/helpers/variables.test.ts b/src/lib/helpers/variables.test.ts new file mode 100644 index 0000000000..75f431a88b --- /dev/null +++ b/src/lib/helpers/variables.test.ts @@ -0,0 +1,49 @@ +import { expect, test } from 'vitest'; +import { + getVariableKeyError, + isValidVariableKey, + validateVariables, + VARIABLE_KEY_MAX_LENGTH, + VARIABLE_VALUE_MAX_LENGTH +} from '$lib/helpers/variables'; + +test('accept keys that are valid environment variable names', () => { + expect(isValidVariableKey('APP_TEST')).toBe(true); + expect(isValidVariableKey('_PRIVATE')).toBe(true); + expect(isValidVariableKey('key1')).toBe(true); + expect(isValidVariableKey('a'.repeat(VARIABLE_KEY_MAX_LENGTH))).toBe(true); +}); + +test('reject keys that cannot be used as environment variable names', () => { + expect(isValidVariableKey('MY-KEY')).toBe(false); + expect(isValidVariableKey('MY.KEY')).toBe(false); + expect(isValidVariableKey('MY KEY')).toBe(false); + expect(isValidVariableKey('9KEY')).toBe(false); + expect(isValidVariableKey('KÉY')).toBe(false); + expect(isValidVariableKey('KEY\t')).toBe(false); + expect(isValidVariableKey('')).toBe(false); + expect(isValidVariableKey('a'.repeat(VARIABLE_KEY_MAX_LENGTH + 1))).toBe(false); +}); + +test('report a missing key separately from an invalid one', () => { + expect(getVariableKeyError('')).toEqual('Variable key is required'); + expect(getVariableKeyError('MY-KEY')).toContain('is invalid'); + expect(getVariableKeyError('a'.repeat(VARIABLE_KEY_MAX_LENGTH + 1))).toContain('longer than'); + expect(getVariableKeyError('APP_TEST')).toBeNull(); +}); + +test('validate a list of variables and name the offending key', () => { + expect(validateVariables([{ key: 'APP_TEST', value: 'value' }])).toBeNull(); + expect(validateVariables([{ key: 'APP_TEST', value: '' }])).toBeNull(); + + expect( + validateVariables([ + { key: 'APP_TEST', value: 'value' }, + { key: 'MY-KEY', value: 'value' } + ]) + ).toContain('MY-KEY'); + + expect( + validateVariables([{ key: 'APP_TEST', value: 'v'.repeat(VARIABLE_VALUE_MAX_LENGTH + 1) }]) + ).toContain('APP_TEST'); +}); diff --git a/src/lib/helpers/variables.ts b/src/lib/helpers/variables.ts index 90219499ed..e93e25620f 100644 --- a/src/lib/helpers/variables.ts +++ b/src/lib/helpers/variables.ts @@ -1,5 +1,71 @@ import type { Models } from '@appwrite.io/console'; +export const VARIABLE_KEY_MAX_LENGTH = 255; +export const VARIABLE_VALUE_MAX_LENGTH = 8192; + +/** + * Variable keys become environment variable names at build and runtime, so the + * API only accepts C-style identifiers. Keys stored before that rule existed + * can still fail this check, which is why `isValidVariableKey` is also used to + * flag them in the variables table. + */ +const VARIABLE_KEY_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/; + +export function isValidVariableKey(key: string): boolean { + return ( + typeof key === 'string' && + key.length <= VARIABLE_KEY_MAX_LENGTH && + VARIABLE_KEY_PATTERN.test(key) + ); +} + +export function getVariableKeyError(key: string): string | null { + if (!key) { + return 'Variable key is required'; + } + + if (key.length > VARIABLE_KEY_MAX_LENGTH) { + return `Variable key "${key}" is longer than ${VARIABLE_KEY_MAX_LENGTH} allowed characters`; + } + + if (!VARIABLE_KEY_PATTERN.test(key)) { + return `Variable key "${key}" is invalid. Keys can only contain letters, digits and underscores, and cannot start with a digit`; + } + + return null; +} + +export function getVariableValueError(key: string, value: unknown): string | null { + if (('' + (value ?? '')).length > VARIABLE_VALUE_MAX_LENGTH) { + return `Variable ${key} is longer than ${VARIABLE_VALUE_MAX_LENGTH} allowed characters`; + } + + return null; +} + +/** + * Returns the first problem found, or null when every variable is accepted by + * the API. Call before submitting so a rejected key is reported against the + * file or row it came from instead of as a bare server error. + */ +export function validateVariables( + variables: Array<{ key?: string; value?: unknown }> +): string | null { + for (const { key, value } of variables) { + const keyError = getVariableKeyError(key); + if (keyError) { + return keyError; + } + + const valueError = getVariableValueError(key, value); + if (valueError) { + return valueError; + } + } + + return null; +} + export function normalizeDetectedVariables( detected: Models.DetectionVariable[] = [] ): Partial[] { diff --git a/src/routes/(console)/project-[region]-[project]/createVariableModal.svelte b/src/routes/(console)/project-[region]-[project]/createVariableModal.svelte index 2696db0fb8..4136fcb186 100644 --- a/src/routes/(console)/project-[region]-[project]/createVariableModal.svelte +++ b/src/routes/(console)/project-[region]-[project]/createVariableModal.svelte @@ -9,12 +9,14 @@ import { Alert, Layout, Selector, Button as PinkButton, Icon } from '@appwrite.io/pink-svelte'; import { Link } from '$lib/elements'; import { IconPlus, IconX } from '@appwrite.io/pink-icons-svelte'; + import { validateVariables } from '$lib/helpers/variables'; export let isGlobal: boolean; export let product: 'function' | 'site' = 'function'; export let showCreate = false; let newVariables: Partial[] = [{ key: '', value: '' }]; let secret = false; + let error = ''; const dispatch = createEventDispatcher(); @@ -26,12 +28,16 @@ if (secret) { newVariables = newVariables.map((variable) => ({ ...variable, secret: true })); } - newVariables.forEach((variable) => { - if (('' + variable.value).length > 8192) { - throw new Error(`Variable ${variable.key} is longer than 8192 allowed characters`); - } - }); - dispatch('created', newVariables); + + const filled = newVariables.filter((variable) => variable.key || variable.value); + + const validationError = validateVariables(filled); + if (validationError) { + error = validationError; + return; + } + + dispatch('created', filled); close(); } @@ -48,6 +54,7 @@ diff --git a/src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte b/src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte index ea90fd2b99..cb7a36047b 100644 --- a/src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte @@ -25,6 +25,7 @@ import { getLatestTag } from '$lib/helpers/github'; import { writable } from 'svelte/store'; import Link from '$lib/elements/link.svelte'; + import { validateVariables } from '$lib/helpers/variables'; let { data @@ -97,6 +98,13 @@ $isSubmitting = true; try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + if (!latestTag) { latestTag = await getLatestTag(data.repository.owner, data.repository.name); } diff --git a/src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte b/src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte index 1e72962068..f49a5ded72 100644 --- a/src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.svelte @@ -28,6 +28,7 @@ import { humanFileSize } from '$lib/helpers/sizeConvertion'; import { currentPlan } from '$lib/stores/organization'; import { uploader } from '$lib/stores/uploader'; + import { validateVariables } from '$lib/helpers/variables'; export let data; @@ -69,6 +70,13 @@ let func: Models.Function | null = null; try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + func = await sdk.forProject(page.params.region, page.params.project).functions.create({ functionId: id || ID.unique(), name, diff --git a/src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte b/src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte index 0e022aabcc..0444fb1bd9 100644 --- a/src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.svelte @@ -22,7 +22,11 @@ import RepoCard from './repoCard.svelte'; import { getIconFromRuntime } from '$lib/stores/runtimes'; import { regionalConsoleVariables } from '$routes/(console)/project-[region]-[project]/store'; - import { normalizeDetectedVariables, mergeVariables } from '$lib/helpers/variables'; + import { + normalizeDetectedVariables, + mergeVariables, + validateVariables + } from '$lib/helpers/variables'; export let data; @@ -104,6 +108,13 @@ async function create() { try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + const func = await sdk .forProject(page.params.region, page.params.project) .functions.create({ diff --git a/src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte b/src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte index a06b57802f..2a09f68146 100644 --- a/src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.svelte @@ -38,6 +38,7 @@ import { getIconFromRuntime } from '$lib/stores/runtimes'; import { regionalConsoleVariables } from '$routes/(console)/project-[region]-[project]/store'; import type { PageData } from './$types'; + import { validateVariables } from '$lib/helpers/variables'; let { data }: { data: PageData } = $props(); @@ -148,6 +149,15 @@ return; } else { try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables( + variables.map((variable) => ({ key: variable.name, value: variable.value })) + ); + if (validationError) { + throw new Error(validationError); + } + const rt = data.template.runtimes.find((r) => r.name === runtime); const func = await sdk diff --git a/src/routes/(console)/project-[region]-[project]/rawVariableEditor.svelte b/src/routes/(console)/project-[region]-[project]/rawVariableEditor.svelte index cdd8899927..b1c3b79b23 100644 --- a/src/routes/(console)/project-[region]-[project]/rawVariableEditor.svelte +++ b/src/routes/(console)/project-[region]-[project]/rawVariableEditor.svelte @@ -6,6 +6,7 @@ import { addNotification } from '$lib/stores/notifications'; import type { Models } from '@appwrite.io/console'; import { parse } from '$lib/helpers/envfile'; + import { validateVariables } from '$lib/helpers/variables'; import { Icon, InlineCode, Layout, Tabs } from '@appwrite.io/pink-svelte'; import { InputTextarea } from '$lib/elements/forms'; import { @@ -35,7 +36,7 @@ ) => Promise; export let sdkUpdateVariable: ( variableId: string, - key: string, + key: string | undefined, value: string, secret?: boolean ) => Promise; @@ -74,10 +75,10 @@ } function validateEntries(entries: DraftVariable[]) { - for (const { key, value } of entries) { - if (value.length > 8192) { - throw new Error(`Variable ${key} is longer than 8192 allowed characters`); - } + const validationError = validateVariables(entries); + + if (validationError) { + throw new Error(validationError); } } @@ -146,22 +147,30 @@ const editableVariables = variableList.variables.filter((variable) => !variable.secret); const secretVariables = variableList.variables.filter((variable) => variable.secret); - await Promise.all( + const existingKeys = editableVariables.map((variable) => variable.key); + const existingResults = await Promise.allSettled( editableVariables.map(async (variable) => { const newValue = vars[variable.key] ?? null; + // Claim the key up front. A rejected write must not leave it + // behind for the second pass, which would retry it as a new + // variable and send the stored key along with it. + delete vars[variable.key]; + if (newValue === null) { await sdkDeleteVariable(variable.$id); } else if (newValue !== variable.value) { - await sdkUpdateVariable(variable.$id, variable.key, newValue, false); + // The key is unchanged here, so leave it out and let + // the API keep the stored one. + await sdkUpdateVariable(variable.$id, undefined, newValue, false); } - delete vars[variable.key]; }) ); // Add new variables, skipping keys that exist in secret variables - await Promise.all( - Object.keys(vars).map(async (key) => { + const newKeys = Object.keys(vars); + const newResults = await Promise.allSettled( + newKeys.map(async (key) => { const existingVariable = variableList.variables.find( (variable) => variable.key === key ); @@ -178,6 +187,22 @@ await sdkCreateVariable(key, vars[key], false); }) ); + + // Every variable is written on its own, so name the keys that + // failed rather than surfacing a single rejection. + const failed = [ + ...existingResults.map((result, index) => + result.status === 'rejected' ? existingKeys[index] : null + ), + ...newResults.map((result, index) => + result.status === 'rejected' ? newKeys[index] : null + ) + ].filter(Boolean); + + if (failed.length) { + throw new Error(`Failed to update variables: ${failed.join(', ')}`); + } + // Ensure secret variables are preserved variableList.variables = [ ...secretVariables, diff --git a/src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte b/src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte index fa75d27942..85719ebc02 100644 --- a/src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/sites/create-site/deploy/+page.svelte @@ -28,6 +28,7 @@ import { getLatestTag } from '$lib/helpers/github'; import Link from '$lib/elements/link.svelte'; import type { FrameworkAdapterWithStartCommand } from '$lib/stores/sites'; + import { validateVariables } from '$lib/helpers/variables'; let { data @@ -147,6 +148,13 @@ $isSubmitting = true; try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + // Create site with build configuration let site = await sdk.forProject(page.params.region, page.params.project).sites.create({ siteId: id || ID.unique(), diff --git a/src/routes/(console)/project-[region]-[project]/sites/create-site/manual/+page.svelte b/src/routes/(console)/project-[region]-[project]/sites/create-site/manual/+page.svelte index f2da210d04..fd5908ab75 100644 --- a/src/routes/(console)/project-[region]-[project]/sites/create-site/manual/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/sites/create-site/manual/+page.svelte @@ -27,6 +27,7 @@ import Domain from '../domain.svelte'; import { uploader } from '$lib/stores/uploader'; import type { FrameworkAdapterWithStartCommand } from '$lib/stores/sites'; + import { validateVariables } from '$lib/helpers/variables'; export let data; let showExitModal = false; @@ -60,6 +61,13 @@ let site: Models.Site | null = null; try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + if (!domainIsValid) { addNotification({ type: 'error', diff --git a/src/routes/(console)/project-[region]-[project]/sites/create-site/repositories/repository-[repository]/+page.svelte b/src/routes/(console)/project-[region]-[project]/sites/create-site/repositories/repository-[repository]/+page.svelte index aca5fe7bb3..70367fabc9 100644 --- a/src/routes/(console)/project-[region]-[project]/sites/create-site/repositories/repository-[repository]/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/sites/create-site/repositories/repository-[repository]/+page.svelte @@ -27,7 +27,11 @@ import Configuration from '../../configuration.svelte'; import Domain from '../../domain.svelte'; import { regionalConsoleVariables } from '$routes/(console)/project-[region]-[project]/store'; - import { normalizeDetectedVariables, mergeVariables } from '$lib/helpers/variables'; + import { + normalizeDetectedVariables, + mergeVariables, + validateVariables + } from '$lib/helpers/variables'; import type { FrameworkAdapterWithStartCommand } from '$lib/stores/sites'; export let data; @@ -113,6 +117,13 @@ return; } try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables(variables); + if (validationError) { + throw new Error(validationError); + } + const fr = Object.values(Framework).find((f) => f === framework.key); const buildRuntime = Object.values(BuildRuntime).find( (f) => f === framework.buildRuntime diff --git a/src/routes/(console)/project-[region]-[project]/sites/create-site/templates/template-[template]/+page.svelte b/src/routes/(console)/project-[region]-[project]/sites/create-site/templates/template-[template]/+page.svelte index a11121d55c..006003d846 100644 --- a/src/routes/(console)/project-[region]-[project]/sites/create-site/templates/template-[template]/+page.svelte +++ b/src/routes/(console)/project-[region]-[project]/sites/create-site/templates/template-[template]/+page.svelte @@ -45,6 +45,7 @@ import { getFrameworkIcon } from '$lib/stores/sites'; import { regionalConsoleVariables } from '$routes/(console)/project-[region]-[project]/store'; import { getTemplateSourceUrl } from '$lib/helpers/templateSource'; + import { validateVariables } from '$lib/helpers/variables'; export let data; @@ -117,6 +118,15 @@ return; } else { try { + // Reject an unusable key before the resource is created, so a + // rejected variable can't leave a half-configured resource behind. + const validationError = validateVariables( + variables.map((variable) => ({ key: variable.name, value: variable.value })) + ); + if (validationError) { + throw new Error(validationError); + } + const fr = Object.values(Framework).find((f) => f === framework.key); const buildRuntime = Object.values(BuildRuntime).find( (f) => f === framework.buildRuntime diff --git a/src/routes/(console)/project-[region]-[project]/updateVariables.svelte b/src/routes/(console)/project-[region]-[project]/updateVariables.svelte index 8b9f3da47e..7164be11a8 100644 --- a/src/routes/(console)/project-[region]-[project]/updateVariables.svelte +++ b/src/routes/(console)/project-[region]-[project]/updateVariables.svelte @@ -5,6 +5,7 @@ import { CardGrid, Empty, Output, PaginationInline } from '$lib/components'; import UploadVariables from './uploadVariablesModal.svelte'; import { variablesOperation, type VariablesOperationItem } from './variablesOperation'; + import { isValidVariableKey, validateVariables } from '$lib/helpers/variables'; import { goto, invalidate } from '$app/navigation'; import { Click, Submit, trackError, trackEvent } from '$lib/actions/analytics'; import { Dependencies } from '$lib/constants'; @@ -21,6 +22,7 @@ Layout, Popover, Table, + Tooltip, Alert } from '@appwrite.io/pink-svelte'; import { @@ -55,7 +57,7 @@ ) => Promise; export let sdkUpdateVariable: ( variableId: string, - key: string, + key: string | undefined, value: string, secret?: boolean ) => Promise; @@ -127,10 +129,24 @@ async function handleVariableCreated(event: CustomEvent) { const variables = event.detail; try { - const promises = variables.map((variable) => - sdkCreateVariable(variable.key, variable.value, variable?.secret || false) + const results = await Promise.allSettled( + variables.map((variable) => + sdkCreateVariable(variable.key, variable.value, variable?.secret || false) + ) ); - await Promise.all(promises); + + // Each variable is created on its own, so name the keys that failed + // instead of reporting a single rejection for the whole batch. + const failed = results + .map((result, index) => + result.status === 'rejected' ? variables[index].key : null + ) + .filter(Boolean); + + if (failed.length) { + throw new Error(`Failed to create variables: ${failed.join(', ')}`); + } + fullVariableList = undefined; showVariablesModal = false; addNotification({ @@ -174,7 +190,10 @@ async function handleVariableSecret(event: CustomEvent) { const variable = event.detail; try { - await sdkUpdateVariable(variable.$id, variable.key, variable.value, variable.secret); + // Marking a variable secret never changes its key, so leave the key + // out and keep the stored one — including keys that predate the + // identifier rule. + await sdkUpdateVariable(variable.$id, undefined, variable.value, variable.secret); fullVariableList = undefined; selectedVar = null; showVariablesModal = false; @@ -237,6 +256,14 @@ async function handleVariablePromoted(variable: Models.Variable) { try { + // Promoting a conflicting key deletes the existing global variable + // before recreating it, so refuse a key the API would reject rather + // than deleting one and failing to create its replacement. + const validationError = validateVariables([variable]); + if (validationError) { + throw new Error(validationError); + } + const globalVariable = globalVariableList ? globalVariableList.variables.find( (globalVariable) => globalVariable.key === variable.key @@ -507,6 +534,19 @@ class="icon-exclamation u-color-text-warning" aria-hidden="true"> {/if} + {#if !isValidVariableKey(variable.key)} + + + + This key can't be used as an environment variable + name, so it is ignored at build and runtime. Rename + it using only letters, digits and underscores, + without starting with a digit. + + + {/if} {variable.key} diff --git a/src/routes/(console)/project-[region]-[project]/updateVariablesModal.svelte b/src/routes/(console)/project-[region]-[project]/updateVariablesModal.svelte index d22a14c57b..af61ff9fa0 100644 --- a/src/routes/(console)/project-[region]-[project]/updateVariablesModal.svelte +++ b/src/routes/(console)/project-[region]-[project]/updateVariablesModal.svelte @@ -8,6 +8,7 @@ import { base } from '$app/paths'; import { Alert, Layout, Selector } from '@appwrite.io/pink-svelte'; import { Link } from '$lib/elements'; + import { getVariableValueError, validateVariables } from '$lib/helpers/variables'; export let isGlobal: boolean; export let product: 'function' | 'site' = 'function'; @@ -21,6 +22,10 @@ secret: selectedVar?.secret }; + const originalKey = selectedVar?.key; + + let error = ''; + const dispatch = createEventDispatcher(); function close() { @@ -29,7 +34,21 @@ } function handleVariable() { - dispatch('updated', pair); + const keyChanged = pair.key !== originalKey; + + // A key stored before the identifier rule existed is left untouched so + // its value stays editable; only a key the user actually changed has to + // satisfy the rule. + const validationError = keyChanged + ? validateVariables([pair]) + : getVariableValueError(pair.key, pair.value); + + if (validationError) { + error = validationError; + return; + } + + dispatch('updated', { ...pair, key: keyChanged ? pair.key : undefined }); close(); } @@ -41,6 +60,7 @@ diff --git a/src/routes/(console)/project-[region]-[project]/uploadVariablesModal.svelte b/src/routes/(console)/project-[region]-[project]/uploadVariablesModal.svelte index 134fe5c09c..e2d10734c0 100644 --- a/src/routes/(console)/project-[region]-[project]/uploadVariablesModal.svelte +++ b/src/routes/(console)/project-[region]-[project]/uploadVariablesModal.svelte @@ -16,6 +16,7 @@ } from '@appwrite.io/pink-svelte'; import { parse } from '$lib/helpers/envfile'; import { removeFile } from '$lib/helpers/files'; + import { validateVariables } from '$lib/helpers/variables'; import type { VariablesOperationItem } from './variablesOperation'; export let show = false; @@ -27,7 +28,7 @@ ) => Promise; export let sdkUpdateVariable: ( variableId: string, - key: string, + key: string | undefined, value: string, secret?: boolean ) => Promise; @@ -64,18 +65,19 @@ const entries = Object.entries(uploaded); - for (const [key, value] of entries) { - if (value.length > 8192) { - throw new Error(`Variable ${key} is longer than 8192 allowed characters`); - } - } - const filteredEntries = entries.filter(([, value]) => !!value); if (!filteredEntries.length) { throw new Error('No variables found'); } + const validationError = validateVariables( + filteredEntries.map(([key, value]) => ({ key, value })) + ); + if (validationError) { + throw new Error(validationError); + } + if (filteredEntries.length > 100) { throw new Error('Please upload a file with fewer than 100 environment variables.'); } @@ -93,7 +95,7 @@ show = false; - await Promise.all( + const results = await Promise.allSettled( filteredEntries.map(([key, value]) => { const found = variableList.variables.find((variable) => variable.key === key); return found @@ -102,6 +104,18 @@ }) ); + // Each variable is written on its own, so report which keys failed + // instead of letting one rejection hide the ones that landed. + const failed = results + .map((result, index) => + result.status === 'rejected' ? filteredEntries[index][0] : null + ) + .filter(Boolean); + + if (failed.length) { + throw new Error(`Failed to upload variables: ${failed.join(', ')}`); + } + onStatusChange({ id: importId, count: uploadCount,