0.0.6

Reviewed-on: #49

.gitea/workflows/lints.yml

@@ -0,0 +1,24 @@
+on:
+  workflow_call:
+  push:
+    branches:
+      - main
+  pull_request:
+  schedule:
+    - cron: '0 0 * * *'
+
+jobs:
+  shellcheck:
+    name: Misc Linters
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout sources
+        uses: https://gitea.com/actions/checkout@v4
+
+      - name: Lint shellscripts
+        uses: ludeeus/action-shellcheck@master
+
+      - name: Lint Dockerfile
+        uses: hadolint/hadolint-action@v3.1.0
+        with:
+          dockerfile: ./Containerfile

.gitea/workflows/publish.yml

@@ -6,6 +6,9 @@ on:
       - 'v*'
 
 jobs:
+  lints:
+    uses: ./.gitea/workflows/lints.yml
+
   tests:
     uses: ./.gitea/workflows/node.yml
 
@@ -21,7 +24,7 @@ jobs:
       - name: Checkout sources
         uses: actions/checkout@v4
 
-      - name: Login to DockerHub
+      - name: Login to Container Registry
         uses: docker/login-action@v3
         with:
           registry: https://git.vbrandl.net

.prettierrc

@@ -1,7 +1,7 @@
 {
 	"useTabs": true,
 	"singleQuote": true,
-	"trailingComma": "none",
+	"trailingComma": "es5",
 	"printWidth": 100,
 	"plugins": ["prettier-plugin-svelte"],
 	"overrides": [{ "files": "*.svelte", "options": { "parser": "svelte" } }]

Containerfile

@@ -1,4 +1,4 @@
-FROM node:21-alpine AS builder
+FROM node:22-alpine AS builder
 
 WORKDIR /app
 COPY package.json package-lock.json ./
@@ -7,9 +7,12 @@ RUN npm ci
 COPY . .
 RUN npm run build
 
-FROM node:21-alpine
+FROM node:22-alpine
-USER node:node
 WORKDIR /app
+RUN chown -R node:node /app
+USER node:node
+COPY ./container/entrypoint.sh /entrypoint.sh
 COPY package.json .
+COPY --from=builder /app/node_modules ./node_modules
 COPY --from=builder /app/build ./build
-CMD ["node", "./build/index.js"]
+ENTRYPOINT ["sh", "/entrypoint.sh"]

container/entrypoint.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env sh
+
+set -e
+
+export STORAGE_PATH="${STORAGE_PATH:-./uploads}"
+
+entrypoint() {
+  mkdir -p "${STORAGE_PATH}"
+  node ./build/index.js
+}
+
+entrypoint

eslint.config.js

@@ -15,19 +15,19 @@ export default [
 		languageOptions: {
 			globals: {
 				...globals.browser,
-				...globals.node
+				...globals.node,
-			}
+			},
-		}
+		},
 	},
 	{
 		files: ['**/*.svelte'],
 		languageOptions: {
 			parserOptions: {
-				parser: ts.parser
+				parser: ts.parser,
-			}
+			},
-		}
+		},
 	},
 	{
-		ignores: ['build/', '.svelte-kit/', 'dist/']
+		ignores: ['build/', '.svelte-kit/', 'dist/'],
-	}
+	},
 ];

flake.lock

@@ -20,11 +20,11 @@
 		},
 		"nixpkgs": {
 			"locked": {
-				"lastModified": 1720957393,
+				"lastModified": 1723637854,
-				"narHash": "sha256-oedh2RwpjEa+TNxhg5Je9Ch6d3W1NKi7DbRO1ziHemA=",
+				"narHash": "sha256-med8+5DSWa2UnOqtdICndjDAEjxr5D7zaIiK4pn0Q7c=",
 				"owner": "nixos",
 				"repo": "nixpkgs",
-				"rev": "693bc46d169f5af9c992095736e82c3488bf7dbb",
+				"rev": "c3aa7b8938b17aebd2deecf7be0636000d62a2b9",
 				"type": "github"
 			},
 			"original": {

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "fotochallenge",
-	"version": "0.0.1",
+	"version": "0.0.6",
 	"private": true,
 	"scripts": {
 		"dev": "vite dev",
@@ -17,8 +17,10 @@
 		"@sveltejs/adapter-node": "^5.2.0",
 		"@sveltejs/kit": "^2.0.0",
 		"@sveltejs/vite-plugin-svelte": "^3.0.0",
+		"@types/bunyan": "^1.8.11",
 		"@types/eslint": "^9.0.0",
 		"@types/node": "^20.14.11",
+		"@types/uuid": "^10.0.0",
 		"bulma": "^1.0.1",
 		"eslint": "^9.0.0",
 		"eslint-config-prettier": "^9.1.0",
@@ -39,5 +41,9 @@
 	"type": "module",
 	"engines": {
 		"node": ">20"
+	},
+	"dependencies": {
+		"bunyan": "^1.8.15",
+		"uuid": "^10.0.0"
 	}
 }

renovate.json5

@@ -1,4 +1,4 @@
 {
 	$schema: 'https://docs.renovatebot.com/renovate-schema.json',
-	extends: ['local>renovate-bot/renovate-config']
+	extends: ['local>renovate-bot/renovate-config'],
 }

src/app.d.ts

@@ -9,3 +9,9 @@ declare global {
 		// interface Platform {}
 	}
 }
+
+declare namespace App {
+	interface Locals {
+		requestId: string;
+	}
+}

src/app.html

@@ -4,6 +4,7 @@
 		<meta charset="utf-8" />
 		<link rel="icon" href="%sveltekit.assets%/favicon.png" />
 		<meta name="viewport" content="width=device-width, initial-scale=1" />
+		<title>Gabi und Hannes Fotochallenge</title>
 		%sveltekit.head%
 	</head>
 	<body data-sveltekit-preload-data="hover">

src/hooks.server.ts

@@ -0,0 +1,45 @@
+import { log, timedExecution } from '$lib';
+import { validate, v7 as uuidv7 } from 'uuid';
+
+const requestIdHeader = 'x-request-id';
+
+/** @type {import('@sveltejs/kit').Handle} */
+export async function handle({ event, resolve }) {
+	// use incomming requestId, if it is a valid uuid, else generate one
+	const reqIdFromRequest = event.request.headers.get(requestIdHeader);
+	const { requestId, fromRequest } =
+		reqIdFromRequest && validate(reqIdFromRequest)
+			? { requestId: reqIdFromRequest, fromRequest: true }
+			: { requestId: uuidv7(), fromRequest: false };
+
+	const context = {
+		requestId,
+		route: event.route.id,
+		method: event.request.method,
+		userAgent: event.request.headers.get('user-agent'),
+		clientIP: event.getClientAddress(),
+	};
+	if (fromRequest) {
+		log.trace(context, 'using incomming request-id');
+	}
+
+	// make requestId available to handlers
+	event.locals.requestId = requestId;
+
+	const { executionTime, result: response } = await timedExecution(async () => {
+		return await resolve(event);
+	});
+	response.headers.set(requestIdHeader, requestId);
+
+	log.info(
+		{
+			executionTime: `${executionTime}ms`,
+			status: response.status,
+			size: response.headers.get('content-length'),
+			...context,
+		},
+		'finished request'
+	);
+
+	return response;
+}

src/index.test.ts

@@ -2,7 +2,31 @@ import safePath from '$lib';
 import { describe, it, expect } from 'vitest';
 
 describe('safe path', () => {
-	it('removes non alphanum from string', () => {
+	it('reject names with ../', () => {
-		expect(safePath('../../!=-.,/abc123')).toBe('abc123');
+		expect(safePath('./uplodas', '../foobar')).toBe(false);
+	});
+
+	it('accept names with ./', () => {
+		expect(safePath('./uplodas', './foobar')).toBe(true);
+	});
+
+	it('reject names with /', () => {
+		expect(safePath('./uplodas', 'foo/bar')).toBe(false);
+	});
+
+	it('accept happy path', () => {
+		expect(safePath('./uplodas', 'foobar')).toBe(true);
+	});
+
+	it('accept names starting with `..`', () => {
+		expect(safePath('./uplodas', '..foobar')).toBe(true);
+	});
+
+	it('accept names ending with `..`', () => {
+		expect(safePath('./uplodas', 'foobar..')).toBe(true);
+	});
+
+	it('accept names starting and ending with `..`', () => {
+		expect(safePath('./uplodas', '..foobar..')).toBe(true);
 	});
 });

src/lib/index.ts

@@ -1,6 +1,42 @@
 // place files you want to import through the `$lib` alias in this folder.
+import path from 'path';
+import bunyan from 'bunyan';
+import type { MaybePromise } from '@sveltejs/kit';
 
-const safePath = (input: string) => input.replace(/\W/g, '');
+export const log = bunyan.createLogger({
-export const storagePath: string = './uploads';
+	name: 'fotochallenge',
+	level: process.env.NODE_ENV === 'production' ? 'info' : 'debug',
+	src: true,
+});
+
+function safePath(basePath: string, name: string): boolean {
+	const fullPath = `${basePath}/${name}`;
+	const relative = path.relative(basePath, fullPath);
+	return (
+		!!relative &&
+		// does move out of `basePath`
+		!relative.startsWith(`..${path.sep}`) &&
+		// exactly one layer deep, e.g. no `./uplodas/foo/bar`
+		!relative.includes(path.sep) &&
+		// result is not an absolute path
+		!path.isAbsolute(relative)
+	);
+}
+
+const defaultPath: string = './uploads';
+if (!('STORAGE_PATH' in process.env)) {
+	log.warn(`'STORAGE_PATH' environment variable is not set. Defaulting to ${defaultPath}`);
+}
+export const storagePath: string = process.env.STORAGE_PATH ?? defaultPath;
 
 export default safePath;
+
+export async function timedExecution<T>(
+	fn: () => MaybePromise<T>
+): Promise<{ executionTime: number; result: T }> {
+	const start = process.hrtime();
+	const result = await fn();
+	const end = process.hrtime(start);
+	const executionTime = (end[0] * 1e6 + end[1]) / 1e6;
+	return { executionTime, result };
+}

src/routes/+page.server.ts

@@ -1,7 +1,7 @@
 import { writeFileSync, mkdirSync, existsSync } from 'fs';
 import { fail } from '@sveltejs/kit';
 import type { RequestEvent } from './$types';
-import safePath, { storagePath } from '$lib';
+import safePath, { storagePath, log } from '$lib';
 import { hash } from 'crypto';
 import path from 'path';
 
@@ -12,29 +12,44 @@ const mkdirIfNotExists = (path: string) => {
 };
 
 export const actions = {
-	default: async ({ request }: RequestEvent) => {
+	default: async ({ request, locals }: RequestEvent) => {
+		let context: { [key: string]: string | string[] } = { requestId: locals.requestId };
 		const data = await request.formData();
 
 		const formFiles = data.getAll('files');
 		if (!formFiles) {
+			log.debug(context, 'missing files');
 			return fail(400, { field: 'files', files: formFiles, missing: true });
 		} else if (!(formFiles as File[])) {
+			log.debug(context, 'invalid files');
 			return fail(400, { field: 'files', files: formFiles, incorrect: true });
 		}
 		const files = formFiles as File[];
-		console.log(files);
+		const fileNames = files.map((file) => file.name);
+		context = { fileNames, ...context };
 		if (files.length === 0) {
+			log.debug(context, 'empty files');
 			return fail(400, { field: 'files', files: formFiles, empty: true });
 		}
 
 		const formName = data.get('name');
 		if (!formName) {
+			log.debug(context, 'missing name');
 			return fail(400, { field: 'name', name: formName, missing: true });
 		} else if (!(formName as string)) {
+			log.debug(context, 'invalid name');
 			return fail(400, { field: 'name', name: formName, incorrect: true });
 		}
 
-		const name = safePath(formName as string);
+		const name = formName as string;
+		context = { name, ...context };
+
+		if (!safePath(storagePath, name)) {
+			log.warn(context, 'Supplied name would cause dir traversal. Rejecting...');
+			return fail(400, { field: 'name', name: name, incorrect: true });
+		}
+
+		log.info(context, 'Uploading files');
 
 		files.forEach(async (file) => {
 			const outPath = `${storagePath}/${name}`;
@@ -42,17 +57,19 @@ export const actions = {
 			const ext = path.extname(file.name);
 
 			mkdirIfNotExists(outPath);
-			const filename = hash('sha1', content);
+			const filename = `${hash('sha1', content)}${ext}`;
-			const fullPath = `${outPath}/${filename}${ext}`;
+			const fullPath = `${outPath}/${filename}`;
+			context = { file: fullPath, ...context };
 			if (existsSync(fullPath)) {
-				console.warn(`${fullPath} has already been uploaded. Skipping...`);
+				log.debug(context, 'File has already been uploaded. Skipping...');
 			} else {
+				log.debug(context, 'saving file');
 				writeFileSync(fullPath, Buffer.from(await file.arrayBuffer()), { flag: 'a+' });
 			}
 		});
 
 		return {
-			success: true
+			success: true,
 		};
-	}
+	},
 };

src/routes/+page.svelte

@@ -13,7 +13,7 @@
 
 	const siPrefixes = new Map([
 		[1_000_000, 'M'],
-		[1_000, 'k']
+		[1_000, 'k'],
 	]);
 	const fileSize = (files: FileList) => {
 		const size = Array.from(files)

svelte.config.js

@@ -11,8 +11,8 @@ const config = {
 		// adapter-auto only supports some environments, see https://kit.svelte.dev/docs/adapter-auto for a list.
 		// If your environment is not supported, or you settled on a specific environment, switch out the adapter.
 		// See https://kit.svelte.dev/docs/adapters for more information about adapters.
-		adapter: adapter()
+		adapter: adapter(),
-	}
+	},
 };
 
 export default config;

vite.config.ts

@@ -5,14 +5,14 @@ export default defineConfig({
 	plugins: [sveltekit()],
 
 	test: {
-		include: ['src/**/*.{test,spec}.{js,ts}']
+		include: ['src/**/*.{test,spec}.{js,ts}'],
 	},
 
 	css: {
 		preprocessorOptions: {
 			scss: {
-				additionalData: '@use "src/variables.scss" as *;'
+				additionalData: '@use "src/variables.scss" as *;',
-			}
+			},
-		}
+		},
-	}
+	},
 });