Update CI builds

.github/workflows/ci.yml

@@ -7,11 +7,13 @@ on:
       - 'main'
     tags:
       - 'v*'
-  pull_request:
+  pull_request_target:
+    types: [opened, synchronize, reopened]
 
 permissions:
   contents: read
   packages: write
+  # pull_request_target has secrets access by default; restrict everything
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
@@ -24,7 +26,11 @@ env:
 jobs:
   test:
     name: Test & Lint
+    if: github.event_name == 'push' || github.event.pull_request.author_association == 'OWNER' || github.event.pull_request.author_association == 'MEMBER' || github.event.pull_request.author_association == 'COLLABORATOR'
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      # No secrets or write access for PRs
     steps:
       - name: Harden the runner (Audit all outbound calls)
         uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0
@@ -33,6 +39,9 @@ jobs:
 
       - name: Checkout repository
         uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
+        with:
+          # Prevent PRs from using credentials - mitigates RCE token exfiltration
+          persist-credentials: false
 
       - name: Setup Node.js
         uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f # v6.1.0
@@ -48,12 +57,17 @@ jobs:
       - name: Lint
         run: npm run lint
 
+        # TODO: duplicate build step; consider removing from here and only keeping in build-and-push
+        # once we figure a better way to prevent build errors from reaching build-and-push job.
+      - name: Build
+        run: npm run build
+
       - name: Test
         run: npm run test
 
   build-and-push:
     name: Build & Push Docker Image
-    if: github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')
+    if: (github.event_name == 'push' || github.event.pull_request.author_association == 'OWNER' || github.event.pull_request.author_association == 'MEMBER' || github.event.pull_request.author_association == 'COLLABORATOR') && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/'))
     needs: test
     runs-on: ubuntu-latest
     permissions:
@@ -106,7 +120,7 @@ jobs:
     name: Create GitHub Release
     needs: build-and-push
     runs-on: ubuntu-latest
-    if: startsWith(github.ref, 'refs/tags/')
+    if: (github.event_name == 'push' || github.event.pull_request.author_association == 'OWNER' || github.event.pull_request.author_association == 'MEMBER' || github.event.pull_request.author_association == 'COLLABORATOR') && startsWith(github.ref, 'refs/tags/')
     steps:
       - name: Harden the runner (Audit all outbound calls)
         uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0

src/api/middleware/errorHandler.ts

@@ -31,26 +31,37 @@ export function errorHandler(
 ): void {
     const errorId = generateErrorId();
 
+    // Extract error properties with safe type handling.
+    // We accept Error | unknown and need to safely access properties that may exist
+    // on Error objects (message), HTTP error objects (statusCode, status), or custom
+    // error objects (code). Type casting to any is necessary to access these
+    // arbitrary properties while maintaining runtime safety through optional chaining.
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const errorObj = err as any; // Allow accessing arbitrary properties
+    const errorMessage = errorObj?.message ?? String(err);
+    const errorType = errorObj?.constructor?.name ?? 'Unknown';
+    const statusCode = errorObj?.statusCode ?? errorObj?.status ?? 500;
+    const errorCode = errorObj?.code;
+
     // Log the full error internally with correlation ID (safe location)
     log.error({
         message: 'API error',
         data: {
             errorId,
-            type: err?.constructor?.name || 'Unknown',
-            message: err?.message || String(err)
+            type: errorType,
+            message: errorMessage
         }
     });
 
     // Return safe error response to client with error ID for tracing
-    const statusCode = err?.statusCode || err?.status || 500;
     const errorResponse: ErrorResponse = {
         error: 'An error occurred processing your request',
         errorId
     };
 
     // Add code if it's a validation error or known error type
-    if (err?.code) {
-        errorResponse.code = err.code;
+    if (errorCode) {
+        errorResponse.code = errorCode;
     }
 
     res.status(statusCode).json(errorResponse);

src/api/middleware/logging.ts

@@ -11,9 +11,18 @@ export function requestLogging(req: Request, res: Response, next: NextFunction):
     const startTime = Date.now();
     const clientIp = getClientIP(req);
 
-    // Override res.end to capture response
-    const originalEnd = res.end;
-    res.end = function (chunk?: Buffer | string, encoding?: string): Response {
+    // Override res.end to capture response timings and metadata.
+    // Express Response.end has multiple overloaded signatures:
+    //   end(): Response
+    //   end(callback: Function): Response
+    //   end(data: Buffer | string): Response
+    //   end(data: Buffer | string, callback: Function): Response
+    //   end(data: Buffer | string, encoding: string, callback: Function): Response
+    // We accept variadic args to match all overloads while preserving the original
+    // function's ability to handle any combination of parameters.
+    const originalEnd = res.end.bind(res);
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    res.end = function (...args: any[]): Response {
         const duration = Date.now() - startTime;
 
         log.debug({
@@ -27,8 +36,8 @@ export function requestLogging(req: Request, res: Response, next: NextFunction):
             }
         });
 
-        return originalEnd.call(this, chunk, encoding);
-    };
+        return originalEnd(...args);
+    } as typeof res.end;
 
     next();
 }

src/backends/traefik/templateParser.ts

@@ -36,10 +36,13 @@ function buildContext(appName: string, data: XMagicProxyData): Record<string, st
 /**
  * Render a template string by replacing {{ variable }} placeholders.
  * 
+ * Throws an error if any template variables cannot be resolved.
+ * 
  * @param template - The template content with {{ variable }} placeholders
  * @param appName - The application name
  * @param data - The proxy configuration data
  * @returns The rendered template as normalized YAML
+ * @throws Error if unknown template variables are encountered
  */
 export function renderTemplate(template: string, appName: string, data: XMagicProxyData): string {
     const context = buildContext(appName, data);
@@ -49,26 +52,37 @@ export function renderTemplate(template: string, appName: string, data: XMagicPr
         data: { appName, context: { app_name: context.app_name, hostname: context.hostname, target_url: context.target_url } }
     });
 
+    // Track unknown variables
+    const unknownVariables: string[] = [];
+
     // Replace all {{ key }} occurrences
     const rendered = template.replace(VARIABLE_PATTERN, (_match, key: string) => {
         if (key in context) {
             return context[key];
         }
-        // Unknown variable - leave as-is and warn
-        log.warn({ message: 'Unknown template variable', data: { appName, variable: key } });
-        return `{{ ${key} }}`;
+        // Track unknown variable for error reporting
+        unknownVariables.push(key);
+        return _match; // Return original text
     });
 
+    // If there were unknown variables, throw an error
+    if (unknownVariables.length > 0) {
+        const uniqueVars = [...new Set(unknownVariables)];
+        const message = `Template contains unknown variables: ${uniqueVars.join(', ')}`;
+        log.error({ message, data: { appName, unknownVariables: uniqueVars } });
+        throw new Error(message);
+    }
+
     // Parse and re-dump for consistent YAML formatting
     try {
         const parsed = yaml.load(rendered);
         return yaml.dump(parsed, { noRefs: true, skipInvalid: true });
     } catch (err) {
-        // Return raw rendered content if YAML parsing fails
-        log.warn({
+        const message = err instanceof Error ? err.message : String(err);
+        log.error({
             message: 'Template produced invalid YAML',
-            data: { appName, error: err instanceof Error ? err.message : String(err) }
+            data: { appName, error: message }
         });
-        return rendered;
+        throw new Error(`Template produced invalid YAML: ${message}`);
     }
 }

src/backends/traefik/traefik.ts

@@ -41,8 +41,9 @@ async function loadTemplate(templatePath: string): Promise<string> {
 
 /**
  * Creates a Traefik config fragment by rendering the appropriate template.
+ * Returns null if template rendering fails.
  */
-function makeAppConfig(appName: string, data: XMagicProxyData): TraefikConfigYamlFormat {
+function makeAppConfig(appName: string, data: XMagicProxyData): TraefikConfigYamlFormat | null {
     lastUserData = data.userData ? JSON.stringify(data.userData) : null;
 
     const templateContent = templates.get(data.template);
@@ -57,10 +58,18 @@ function makeAppConfig(appName: string, data: XMagicProxyData): TraefikConfigYam
         data: { appName, template: data.template, target: data.target, hostname: data.hostname }
     });
 
-    const rendered = renderTemplate(templateContent, appName, data);
-    lastRendered = rendered;
-
-    return yaml.load(rendered) as TraefikConfigYamlFormat;
+    try {
+        const rendered = renderTemplate(templateContent, appName, data);
+        lastRendered = rendered;
+        return yaml.load(rendered) as TraefikConfigYamlFormat;
+    } catch (err) {
+        const message = err instanceof Error ? err.message : String(err);
+        log.error({
+            message: 'Failed to render template',
+            data: { appName, error: message }
+        });
+        return null;
+    }
 }
 
 // ─────────────────────────────────────────────────────────────────────────────
@@ -104,7 +113,7 @@ export async function initialize(config?: MagicProxyConfigFile): Promise<void> {
     }
 
     log.info({ message: 'Initializing Traefik backend', data: { templateCount: templatePaths.length } });
-
+    
     // Load all templates concurrently
     const loadResults = await Promise.all(
         templatePaths.map(async (templatePath) => ({
@@ -138,6 +147,7 @@ export async function initialize(config?: MagicProxyConfigFile): Promise<void> {
 
 /**
  * Add or update a proxied application.
+ * If template rendering fails, the host is skipped with an error log.
  */
 export async function addProxiedApp(entry: HostEntry): Promise<void> {
     const { containerName, xMagicProxy } = entry;
@@ -146,7 +156,16 @@ export async function addProxiedApp(entry: HostEntry): Promise<void> {
         data: { containerName, hostname: xMagicProxy.hostname, target: xMagicProxy.target, template: xMagicProxy.template }
     });
 
-    manager.register(containerName, makeAppConfig(containerName, xMagicProxy));
+    const config = makeAppConfig(containerName, xMagicProxy);
+    if (config === null) {
+        log.error({
+            message: 'Skipping host due to template rendering failure',
+            data: { containerName, hostname: xMagicProxy.hostname }
+        });
+        return;
+    }
+
+    manager.register(containerName, config);
     await manager.flushToDisk();
 }
 

test/unit/providers/docker-provider.test.ts

@@ -11,7 +11,8 @@ vi.mock('../../../src/providers/docker/compose', async () => {
     const actual = await vi.importActual('../../../src/providers/docker/compose');
     return {
         ...actual,
-        groupContainersByComposeFile: vi.fn()
+        groupContainersByComposeFile: vi.fn(),
+        resolveHostPath: vi.fn((path) => path)
     };
 });
 vi.mock('../../../src/providers/docker/manifest', async () => {