feat: Validate Vercel cron paths (#9145)

* feat: Validate Vercel cron paths * fix: Don't make Rich mad with bad variable names, changeset * Update .changeset/tiny-deers-complain.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * fix: Better warning message + semantics * feat: Add type differentiator for endpoints / pages * fix: Better differentiators * feat: Add endpoint and page methods to adapter contract * create ServerMetadataRoute interface * simplify * simplify * move validation into separate function * flesh out error message * endpoint -> api * endpoint -> api --------- Co-authored-by: Rich Harris <richard.a.harris@gmail.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <hello@rich-harris.dev>
sveltejs · Feb 28, 2023 · 1f4b87e · 1f4b87e
1 parent 6ebb6f7
commit 1f4b87e
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 26 deletions.
diff --git a/.changeset/tiny-deers-complain.md b/.changeset/tiny-deers-complain.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/adapter-vercel': minor
+---
+
+feat: validate that Vercel cron paths match an API path
diff --git a/packages/adapter-vercel/index.js b/packages/adapter-vercel/index.js
@@ -39,6 +39,12 @@ const plugin = function (defaults = {}) {
 			builder.rimraf(dir);
 			builder.rimraf(tmp);
 
+			if (fs.existsSync('vercel.json')) {
+				const vercel_file = fs.readFileSync('vercel.json', 'utf-8');
+				const vercel_config = JSON.parse(vercel_file);
+				validate_vercel_json(builder, vercel_config);
+			}
+
 			const files = fileURLToPath(new URL('./files', import.meta.url).href);
 
 			const dirs = {
@@ -499,4 +505,57 @@ async function create_function_bundle(builder, entry, dir, config) {
 	write(`${dir}/package.json`, JSON.stringify({ type: 'module' }));
 }
 
+/**
+ *
+ * @param {import('@sveltejs/kit').Builder} builder
+ * @param {any} vercel_config
+ */
+function validate_vercel_json(builder, vercel_config) {
+	if (builder.routes.length > 0 && !builder.routes[0].api) {
+		// bail — we're on an older SvelteKit version that doesn't
+		// populate `route.api.methods`, so we can't check
+		// to see if cron paths are valid
+		return;
+	}
+
+	const crons = /** @type {Array<unknown>} */ (
+		Array.isArray(vercel_config?.crons) ? vercel_config.crons : []
+	);
+
+	/** For a route to be considered 'valid', it must be an API route with a GET handler */
+	const valid_routes = builder.routes.filter((route) => route.api.methods.includes('GET'));
+
+	/** @type {Array<string>} */
+	const unmatched_paths = [];
+
+	for (const cron of crons) {
+		if (typeof cron !== 'object' || cron === null || !('path' in cron)) {
+			continue;
+		}
+
+		const { path } = cron;
+		if (typeof path !== 'string') {
+			continue;
+		}
+
+		for (const route of valid_routes) {
+			if (route.pattern.test(path)) {
+				continue;
+			}
+		}
+
+		unmatched_paths.push(path);
+	}
+
+	builder.log.warn(
+		`\nvercel.json defines cron tasks that use paths that do not correspond to an API route with a GET handler (ignore this if the request is handled in your \`handle\` hook):`
+	);
+
+	for (const path of unmatched_paths) {
+		console.log(`    - ${path}`);
+	}
+
+	console.log('');
+}
+
 export default plugin;
diff --git a/packages/kit/src/core/adapt/builder.js b/packages/kit/src/core/adapt/builder.js
@@ -43,14 +43,15 @@ export function create_builder({
 	 * we expose a stable type that adapters can use to group/filter routes
 	 */
 	const routes = route_data.map((route) => {
-		const methods =
-			/** @type {import('types').HttpMethod[]} */
-			(server_metadata.routes.get(route.id)?.methods);
-		const config = server_metadata.routes.get(route.id)?.config;
+		const { config, methods, page, api } = /** @type {import('types').ServerMetadataRoute} */ (
+			server_metadata.routes.get(route.id)
+		);
 
 		/** @type {import('types').RouteDefinition} */
 		const facade = {
 			id: route.id,
+			api,
+			page,
 			segments: get_route_segments(route.id).map((segment) => ({
 				dynamic: segment.includes('['),
 				rest: segment.includes('[...'),

diff --git a/packages/kit/src/core/postbuild/analyse.js b/packages/kit/src/core/postbuild/analyse.js
@@ -62,8 +62,11 @@ async function analyse({ manifest_path, env }) {
 
 	// analyse routes
 	for (const route of manifest._.routes) {
-		/** @type {Set<import('types').HttpMethod>} */
-		const methods = new Set();
+		/** @type {Array<'GET' | 'POST'>} */
+		const page_methods = [];
+
+		/** @type {import('types').HttpMethod[]} */
+		const api_methods = [];
 
 		/** @type {import('types').PrerenderOption | undefined} */
 		let prerender = undefined;
@@ -84,12 +87,12 @@ async function analyse({ manifest_path, env }) {
 				prerender = mod.prerender;
 			}
 
-			if (mod.GET) methods.add('GET');
-			if (mod.POST) methods.add('POST');
-			if (mod.PUT) methods.add('PUT');
-			if (mod.PATCH) methods.add('PATCH');
-			if (mod.DELETE) methods.add('DELETE');
-			if (mod.OPTIONS) methods.add('OPTIONS');
+			if (mod.GET) api_methods.push('GET');
+			if (mod.POST) api_methods.push('POST');
+			if (mod.PUT) api_methods.push('PUT');
+			if (mod.PATCH) api_methods.push('PATCH');
+			if (mod.DELETE) api_methods.push('DELETE');
+			if (mod.OPTIONS) api_methods.push('OPTIONS');
 
 			config = mod.config;
 		}
@@ -112,8 +115,8 @@ async function analyse({ manifest_path, env }) {
 			}
 
 			if (page) {
-				methods.add('GET');
-				if (page.server?.actions) methods.add('POST');
+				page_methods.push('GET');
+				if (page.server?.actions) page_methods.push('POST');
 
 				validate_page_server_exports(page.server, page.server_id);
 				validate_common_exports(page.universal, page.universal_id);
@@ -133,9 +136,15 @@ async function analyse({ manifest_path, env }) {
 		}
 
 		metadata.routes.set(route.id, {
-			prerender,
 			config,
-			methods: Array.from(methods)
+			methods: Array.from(new Set([...page_methods, ...api_methods])),
+			page: {
+				methods: page_methods
+			},
+			api: {
+				methods: api_methods
+			},
+			prerender
 		});
 	}
 

diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts
@@ -906,7 +906,7 @@ export interface RequestEvent<
 	 */
 	locals: App.Locals;
 	/**
-	 * The parameters of the current page or endpoint - e.g. for a route like `/blog/[slug]`, a `{ slug: string }` object
+	 * The parameters of the current route - e.g. for a route like `/blog/[slug]`, a `{ slug: string }` object
 	 */
 	params: Params;
 	/**
@@ -950,7 +950,7 @@ export interface RequestEvent<
 	 */
 	setHeaders(headers: Record<string, string>): void;
 	/**
-	 * The URL of the current page or endpoint.
+	 * The requested URL.
 	 */
 	url: URL;
 	/**
@@ -997,6 +997,12 @@ export interface ResolveOptions {
 
 export interface RouteDefinition<Config = any> {
 	id: string;
+	api: {
+		methods: HttpMethod[];
+	};
+	page: {
+		methods: Extract<HttpMethod, 'GET' | 'POST'>[];
+	};
 	pattern: RegExp;
 	prerender: PrerenderOption;
 	segments: RouteSegment[];

diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts
@@ -254,16 +254,21 @@ export interface ServerErrorNode {
 	status?: number;
 }
 
+export interface ServerMetadataRoute {
+	config: any;
+	api: {
+		methods: HttpMethod[];
+	};
+	page: {
+		methods: Array<'GET' | 'POST'>;
+	};
+	methods: HttpMethod[];
+	prerender: PrerenderOption | undefined;
+}
+
 export interface ServerMetadata {
 	nodes: Array<{ has_server_load: boolean }>;
-	routes: Map<
-		string,
-		{
-			prerender: PrerenderOption | undefined;
-			methods: HttpMethod[];
-			config: any;
-		}
-	>;
+	routes: Map<string, ServerMetadataRoute>;
 }
 
 export interface SSRComponent {