fix(app-store): harden App Store with auth, validation, bug fixes, and design compliance (#523)

* fix(app-store): harden App Store with auth, validation, bug fixes, and design compliance

Add authMiddleware to GET /api/templates and POST /api/templates/deploy
endpoints. Add isValidStackName and isPathWithinBase checks to the deploy
endpoint. Replace fs.existsSync with async fsPromises.access. Extract
FileSystemService to local variable to avoid repeated getInstance calls.

Fix template mutation bug where PUID/PGID/TZ duplicated on re-open by
working on a copy instead of mutating state. Make env_file conditional
in generated compose YAML (only when env vars exist). Add port validation
(range 1-65535) with visual feedback and deploy blocking. Add env key
collision warning toast for custom variables.

Replace any types with proper LSIO API interfaces. Change catch types
from any to unknown with getErrorMessage. Add structured logging with
[Templates] prefix and diagnostic logging gated behind Developer Mode.

Align with design system: remove hardcoded bg-white and text-red-500,
use ScrollArea, fix destructive button variant, add tabular-nums and
strokeWidth 1.5, use cn() for conditional classes. Add empty-registry
state distinct from no-search-results.

Add 16 unit tests for TemplateService covering compose generation,
conditional env_file, env string generation, and cache clearing.
Update App Store docs with port validation and env collision details.

* refactor(app-store): remove unused interface exports

Remove export keyword from interfaces that are only used within their
own file: TemplateEnv, TemplateVolume, and TemplatesResponse in
TemplateService.ts; TemplateEnv and Template in AppStoreView.tsx.
No external consumers import these types.

* fix(app-store): remove unused fs default import

The fs.existsSync call was replaced with fsPromises.access in the
deploy endpoint, leaving the fs default import unused. Remove it
to fix the ESLint no-unused-vars error in CI.

backend/src/__tests__/template-service.test.ts

@@ -0,0 +1,222 @@
+/**
+ * Unit tests for TemplateService: compose YAML generation,
+ * env string generation, conditional env_file, and cache clearing.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { TemplateService, Template } from '../services/TemplateService';
+
+describe('TemplateService', () => {
+  let service: TemplateService;
+
+  beforeEach(() => {
+    service = new TemplateService();
+  });
+
+  // ─── generateComposeFromTemplate ─────────────────────────────────────
+
+  describe('generateComposeFromTemplate', () => {
+    it('generates minimal compose with just image and restart policy', () => {
+      const template: Template = {
+        title: 'nginx',
+        description: 'Web server',
+        image: 'nginx:latest',
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('image: nginx:latest');
+      expect(yaml).toContain('restart: unless-stopped');
+      expect(yaml).not.toContain('ports:');
+      expect(yaml).not.toContain('volumes:');
+      expect(yaml).not.toContain('env_file:');
+    });
+
+    it('includes ports when template has port mappings', () => {
+      const template: Template = {
+        title: 'nginx',
+        description: 'Web server',
+        image: 'nginx:latest',
+        ports: ['80:80', '443:443/tcp'],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('ports:');
+      expect(yaml).toContain('"80:80"');
+      expect(yaml).toContain('"443:443/tcp"');
+    });
+
+    it('handles string volumes with host:container format', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: ['/host/data:/container/data'],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('volumes:');
+      expect(yaml).toContain('/host/data:/container/data');
+    });
+
+    it('handles string volumes with single path (named volume)', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: ['/data'],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('- /data');
+    });
+
+    it('handles object volumes with container and bind', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: [{ container: '/config', bind: './config' }],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('./config:/config');
+    });
+
+    it('generates bind path from container folder when bind is not specified', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: [{ container: '/app/data' }],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('./data:/app/data');
+    });
+
+    it('adds :ro suffix for readonly volumes', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: [{ container: '/config', bind: './config', readonly: true }],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('./config:/config:ro');
+    });
+
+    it('includes env_file only when env vars are present', () => {
+      const withEnv: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        env: [{ name: 'TZ', default: 'UTC' }],
+      };
+
+      const withoutEnv: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        env: [],
+      };
+
+      expect(service.generateComposeFromTemplate(withEnv)).toContain('env_file:');
+      expect(service.generateComposeFromTemplate(withoutEnv)).not.toContain('env_file:');
+    });
+
+    it('does not include env_file when env is undefined', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+      };
+
+      expect(service.generateComposeFromTemplate(template)).not.toContain('env_file:');
+    });
+
+    it('handles string volumes with options (e.g., host:container:ro)', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: ['/host/config:/config:ro'],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toContain('/host/config:/config:ro');
+    });
+
+    it('skips object volumes without container path', () => {
+      const template: Template = {
+        title: 'app',
+        description: 'Test',
+        image: 'test:latest',
+        volumes: [{ container: '' }],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      // Empty container means `continue` is hit, no volume line emitted
+      expect(yaml).toContain('volumes:');
+      // The volume header is added but no actual volume entry
+      const volumeLines = yaml.split('\n').filter(l => l.trim().startsWith('- '));
+      expect(volumeLines).toHaveLength(0);
+    });
+
+    it('produces valid YAML structure starting with services key', () => {
+      const template: Template = {
+        title: 'full',
+        description: 'Full template',
+        image: 'app:v1',
+        ports: ['8080:80'],
+        volumes: [{ container: '/data', bind: './data' }],
+        env: [{ name: 'KEY', default: 'val' }],
+      };
+
+      const yaml = service.generateComposeFromTemplate(template);
+      expect(yaml).toMatch(/^services:\n/);
+      expect(yaml).toContain('  app:');
+    });
+  });
+
+  // ─── generateEnvString ───────────────────────────────────────────────
+
+  describe('generateEnvString', () => {
+    it('converts key-value pairs to env file format', () => {
+      const result = service.generateEnvString({
+        TZ: 'America/New_York',
+        PUID: '1000',
+        PGID: '1000',
+      });
+
+      expect(result).toBe('TZ=America/New_York\nPUID=1000\nPGID=1000');
+    });
+
+    it('returns empty string for empty object', () => {
+      expect(service.generateEnvString({})).toBe('');
+    });
+
+    it('handles values with special characters', () => {
+      const result = service.generateEnvString({
+        PASSWORD: 'p@ss=word!',
+        URL: 'http://localhost:3000',
+      });
+
+      expect(result).toContain('PASSWORD=p@ss=word!');
+      expect(result).toContain('URL=http://localhost:3000');
+    });
+  });
+
+  // ─── clearCache ──────────────────────────────────────────────────────
+
+  describe('clearCache', () => {
+    it('calls CacheService.invalidate with the correct key', () => {
+      const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+      // clearCache should not throw even when cache is empty
+      expect(() => service.clearCache()).not.toThrow();
+      expect(consoleSpy).toHaveBeenCalledWith('[Templates] Cache invalidated');
+
+      consoleSpy.mockRestore();
+    });
+  });
+});

backend/src/index.ts

@@ -64,7 +64,7 @@ import semver from 'semver';
 import { CronExpressionParser } from 'cron-parser';
 import { isValidStackName, isValidRemoteUrl, isPathWithinBase } from './utils/validation';
 import YAML from 'yaml';
-import fs, { promises as fsPromises } from 'fs';
+import { promises as fsPromises } from 'fs';
 
 // Suppress [DEP0060] DeprecationWarning emitted by http-proxy@1.18.1 which calls
 // util._extend internally. The warning fires at runtime when createProxyServer() is
@@ -5620,11 +5620,12 @@ app.post('/api/system/networks', async (req: Request, res: Response) => {
 
 // --- App Templates Routes ---
 
-app.get('/api/templates', async (req: Request, res: Response) => {
+app.get('/api/templates', authMiddleware, async (req: Request, res: Response) => {
   try {
     const templates = await templateService.getTemplates();
     res.json(templates);
   } catch (error) {
+    console.error('[Templates] Failed to fetch:', error);
     res.status(500).json({ error: 'Failed to fetch templates' });
   }
 });
@@ -5632,10 +5633,11 @@ app.get('/api/templates', async (req: Request, res: Response) => {
 app.post('/api/templates/refresh-cache', authMiddleware, (req: Request, res: Response) => {
   if (!requireAdmin(req, res)) return;
   templateService.clearCache();
+  console.log('[Templates] Cache cleared by', req.user?.username || 'unknown');
   res.json({ success: true });
 });
 
-app.post('/api/templates/deploy', async (req: Request, res: Response) => {
+app.post('/api/templates/deploy', authMiddleware, async (req: Request, res: Response) => {
   if (!requireAdmin(req, res)) return;
   try {
     const { stackName, template, envVars } = req.body;
@@ -5644,26 +5646,42 @@ app.post('/api/templates/deploy', async (req: Request, res: Response) => {
       return res.status(400).json({ error: 'stackName and template are required' });
     }
 
-    const stackPath = path.join(FileSystemService.getInstance(req.nodeId).getBaseDir(), stackName);
-    if (fs.existsSync(stackPath)) {
+    if (!isValidStackName(stackName)) {
+      return res.status(400).json({ error: 'Stack name can only contain alphanumeric characters, hyphens, and underscores' });
+    }
+
+    const fsService = FileSystemService.getInstance(req.nodeId);
+    const baseDir = fsService.getBaseDir();
+    const stackPath = path.join(baseDir, stackName);
+    if (!isPathWithinBase(stackPath, baseDir)) {
+      return res.status(400).json({ error: 'Invalid stack path' });
+    }
+
+    try {
+      await fsPromises.access(stackPath);
       return res.status(409).json({
         error: `A stack directory named '${stackName}' already exists. Please choose a different Stack Name.`,
         rolledBack: false
       });
+    } catch {
+      // Directory does not exist; proceed with deploy
     }
 
+    const debug = isDebugEnabled();
+    console.log(`[Templates] Deploy started: ${stackName}`);
+    if (debug) console.debug('[Templates:debug] Deploy payload', { stackName, templateTitle: template.title, envVarCount: envVars ? Object.keys(envVars).length : 0 });
+
     // 1. Create stack directory
-    await FileSystemService.getInstance(req.nodeId).createStack(stackName);
+    await fsService.createStack(stackName);
 
     // 2. Generate compose YAML and save
     const composeYaml = templateService.generateComposeFromTemplate(template);
-    await FileSystemService.getInstance(req.nodeId).saveStackContent(stackName, composeYaml);
+    await fsService.saveStackContent(stackName, composeYaml);
 
     // 3. Generate env string and save to default .env
-    if (envVars) {
+    if (envVars && Object.keys(envVars).length > 0) {
       const envString = templateService.generateEnvString(envVars);
-      const stackDir = path.join(FileSystemService.getInstance(req.nodeId).getBaseDir(), stackName);
-      const defaultEnvPath = path.join(stackDir, '.env');
+      const defaultEnvPath = path.join(stackPath, '.env');
       await fsPromises.writeFile(defaultEnvPath, envString, 'utf-8');
     }
 
@@ -5672,9 +5690,11 @@ app.post('/api/templates/deploy', async (req: Request, res: Response) => {
       const atomic = LicenseService.getInstance().getTier() === 'paid';
       await ComposeService.getInstance(req.nodeId).deployStack(stackName, terminalWs || undefined, atomic);
       invalidateNodeCaches(req.nodeId);
+      console.log(`[Templates] Deploy completed: ${stackName}`);
       res.json({ success: true, message: 'Template deployed successfully' });
-    } catch (deployError: any) {
-      const rawError = deployError.message || String(deployError);
+    } catch (deployError: unknown) {
+      const rawError = getErrorMessage(deployError, String(deployError));
+      console.error(`[Templates] Deploy failed: ${stackName} -`, rawError);
       const parsed = ErrorParser.parse(rawError);
 
       const shouldRollback = parsed.rule ? parsed.rule.canSilentlyRollback : true;
@@ -5684,14 +5704,14 @@ app.post('/api/templates/deploy', async (req: Request, res: Response) => {
           // Stage 1: Tell Docker to clean up ghost networks/containers
           await ComposeService.getInstance(req.nodeId).downStack(stackName);
         } catch (downErr) {
-          console.error("Rollback Stage 1 (Docker down) failed:", downErr);
+          console.error("[Templates] Rollback Stage 1 (Docker down) failed:", downErr);
         }
 
         try {
-          // Stage 2: Obliterate the files
-          await FileSystemService.getInstance(req.nodeId).deleteStack(stackName);
+          // Stage 2: Remove the stack files
+          await fsService.deleteStack(stackName);
         } catch (fsErr) {
-          console.error("Rollback Stage 2 (File deletion) failed:", fsErr);
+          console.error("[Templates] Rollback Stage 2 (File deletion) failed:", fsErr);
         }
       }
 
@@ -5704,9 +5724,10 @@ app.post('/api/templates/deploy', async (req: Request, res: Response) => {
         ruleId: parsed.rule?.id || 'UNKNOWN'
       });
     }
-  } catch (error: any) {
-    console.error('Failed to deploy template:', error);
-    res.status(500).json({ error: error.message || 'Failed to deploy template' });
+  } catch (error: unknown) {
+    const message = getErrorMessage(error, 'Failed to deploy template');
+    console.error('[Templates] Deploy error:', message);
+    res.status(500).json({ error: message });
   }
 });
 

backend/src/services/TemplateService.ts

@@ -1,15 +1,16 @@
 import axios from 'axios';
 import { DatabaseService } from './DatabaseService';
 import { CacheService } from './CacheService';
+import { isDebugEnabled } from '../utils/debug';
 
 
-export interface TemplateEnv {
+interface TemplateEnv {
     name: string;
     label?: string;
     default?: string;
 }
 
-export interface TemplateVolume {
+interface TemplateVolume {
     container: string;
     bind?: string;
     readonly?: boolean;
@@ -37,11 +38,30 @@ export interface Template {
     };
 }
 
-export interface TemplatesResponse {
+interface TemplatesResponse {
     version: string;
     templates: Template[];
 }
 
+// Typed shapes for the LinuxServer.io API response
+interface LsioPort { external?: number; internal: number; protocol?: string }
+interface LsioVolume { path: string }
+interface LsioEnvVar { name: string; desc?: string; default?: string }
+interface LsioAppConfig { ports?: LsioPort[]; volumes?: LsioVolume[]; environment?: LsioEnvVar[] }
+interface LsioApp {
+    name: string;
+    description?: string;
+    logo?: string;
+    github?: string;
+    readme?: string;
+    arch?: string[];
+    stars?: number;
+    config?: LsioAppConfig;
+}
+interface LsioApiResponse {
+    data?: { repositories?: { linuxserver?: Record<string, LsioApp> } };
+}
+
 // Static category map for LSIO apps (the LSIO API does not expose category metadata).
 // Apps can belong to multiple categories. Unmapped apps fall back to ['Other'].
 const LSIO_CATEGORY_MAP: Record<string, string[]> = {
@@ -197,6 +217,7 @@ export class TemplateService {
 
     public clearCache(): void {
         CacheService.getInstance().invalidate(TemplateService.CACHE_KEY);
+        console.log('[Templates] Cache invalidated');
     }
 
     public async getTemplates(): Promise<Template[]> {
@@ -209,13 +230,15 @@ export class TemplateService {
                     // Default to a reliable LSIO Portainer v2 template registry if not set
                     const registryUrl = settings.template_registry_url || 'https://api.linuxserver.io/api/v1/images?include_config=true';
 
-                    const response = await axios.get<any>(registryUrl);
+                    console.log(`[Templates] Fetching from registry: ${registryUrl}`);
+                    const debug = isDebugEnabled();
 
                     if (registryUrl.includes('api.linuxserver.io')) {
+                        const response = await axios.get<LsioApiResponse>(registryUrl, { timeout: 20_000 });
                         // Official LSIO API Schema Mapping
-                        const lsioApps = response.data?.data?.repositories?.linuxserver || [];
+                        const lsioApps = response.data?.data?.repositories?.linuxserver ?? {};
 
-                        return Object.values(lsioApps).map((app: any) => ({
+                        const templates: Template[] = Object.values(lsioApps).map((app: LsioApp) => ({
                             type: 1,
                             title: app.name,
                             description: app.description || '',
@@ -228,31 +251,39 @@ export class TemplateService {
                             categories: getCategoriesForApp(app.name),
                             source: 'linuxserver',
                             // Map configs if available, otherwise default to empty arrays
-                            ports: (app.config?.ports || []).map((p: any) => `${p.external || p.internal}:${p.internal}/${p.protocol || 'tcp'}`),
-                            volumes: (app.config?.volumes || []).map((v: any) => {
+                            ports: (app.config?.ports ?? []).map((p: LsioPort) => `${p.external || p.internal}:${p.internal}/${p.protocol || 'tcp'}`),
+                            volumes: (app.config?.volumes ?? []).map((v: LsioVolume) => {
                                 const folderName = v.path.split('/').filter(Boolean).pop() || 'data';
                                 return {
                                     container: v.path,
-                                    bind: `./${folderName}` // Proactively create a clean relative path
+                                    bind: `./${folderName}`
                                 };
                             }),
-                            env: (app.config?.environment || []).map((e: any) => ({
+                            env: (app.config?.environment ?? []).map((e: LsioEnvVar) => ({
                                 name: e.name,
                                 label: e.desc || e.name,
                                 default: e.default || ''
                             }))
                         }));
+
+                        console.log(`[Templates] Fetched ${templates.length} templates from LSIO`);
+                        if (debug) console.debug('[Templates:debug] LSIO sample:', templates.slice(0, 5).map(t => t.title));
+                        return templates;
                     }
 
                     // Legacy Portainer v2 Format (Fallback for custom registries)
-                    // The Portainer v2 spec includes a native `categories` field - pass it through.
-                    return (response.data.templates || [])
+                    // The Portainer v2 spec includes a native `categories` field; pass it through.
+                    const response = await axios.get<TemplatesResponse>(registryUrl, { timeout: 20_000 });
+                    const templates = (response.data.templates || [])
                         .filter((t: Template) => !!t.image && t.type === 1)
                         .map((t: Template) => ({ ...t, source: 'custom' }));
+
+                    console.log(`[Templates] Fetched ${templates.length} templates from custom registry`);
+                    return templates;
                 },
             );
         } catch (error) {
-            console.error('Failed to fetch templates', error);
+            console.error('[Templates] Failed to fetch from registry:', error);
             throw new Error('Could not fetch templates from registry');
         }
     }
@@ -304,7 +335,9 @@ export class TemplateService {
             }
         }
 
-        yaml += `    env_file:\n      - .env\n`;
+        if (template.env && template.env.length > 0) {
+            yaml += `    env_file:\n      - .env\n`;
+        }
 
         return yaml;
     }