fix(variables): validate variable keys before writing them

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) {

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<ProductLabel, string> = {
         site: 'https://appwrite.io/docs/products/sites/develop#accessing-environment-variables',
@@ -146,7 +147,28 @@
                             </svelte:fragment>
                             {#each paginatedItems as variable}
                                 <Table.Row.Base {root}>
-                                    <Table.Cell column="key" {root}>{variable.key}</Table.Cell>
+                                    <Table.Cell column="key" {root}>
+                                        <Layout.Stack
+                                            gap="xxs"
+                                            alignItems="center"
+                                            direction="row"
+                                            inline>
+                                            {#if !isValidVariableKey(variable.key)}
+                                                <Tooltip maxWidth="26rem">
+                                                    <span
+                                                        class="icon-exclamation u-color-text-danger"
+                                                        aria-hidden="true"></span>
+                                                    <svelte:fragment slot="tooltip">
+                                                        This key can't be used as an environment
+                                                        variable name. Rename it using only letters,
+                                                        digits and underscores, without starting
+                                                        with a digit.
+                                                    </svelte:fragment>
+                                                </Tooltip>
+                                            {/if}
+                                            {variable.key}
+                                        </Layout.Stack>
+                                    </Table.Cell>
                                     <Table.Cell column="value" {root}>
                                         <!-- TODO: fix max width -->
                                         <div style="max-width: 100%">

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<Models.Variable>[];
@@ -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) {

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<Models.Variable>;
@@ -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 @@
     }
 </script>
 
-<Modal bind:show onSubmit={handleVariable} title="Update variable">
+<Modal bind:show onSubmit={handleVariable} title="Update variable" bind:error>
     <span slot="description">
         Update the environment variable for your {productLabel}. Global variables can be set in
         <Link

src/lib/components/variables/variableEditorModal.svelte

@@ -5,6 +5,7 @@
     import Button from '$lib/elements/forms/button.svelte';
     import { addNotification } from '$lib/stores/notifications';
     import { parse } from '$lib/helpers/envfile';
+    import { validateVariables } from '$lib/helpers/variables';
     import { Alert, Icon, Layout, Tabs } from '@appwrite.io/pink-svelte';
     import { IconDownload, IconDuplicate } from '@appwrite.io/pink-icons-svelte';
     import { InputTextarea } from '$lib/elements/forms';
@@ -47,10 +48,11 @@
             const vars = tab === 'env' ? parse(envCode) : JSON.parse(jsonCode ? jsonCode : '{}');
             const entries = Object.entries(vars);
 
-            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);
             }
 
             // Update or remove editable variables

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');
+});

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<Models.Variable>[] {

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<Models.Variable>[] = [{ 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 @@
 
 <Modal
     bind:show={showCreate}
+    bind:error
     onSubmit={handleVariable}
     title={`Create ${isGlobal ? 'global' : 'environment'} variables`}>
     <svelte:fragment slot="description">

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);
             }

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,