fix: scope the hub pick route as a write and keep an alphabetical floor

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqHuykfRrkDHj9dHCJQQcE

backend/windmill-api/openapi.yaml

@@ -8449,7 +8449,7 @@ paths:
                     - picks
 
   /w/{workspace}/resources/type/hub/pick/{name}:
-    get:
+    post:
       summary: record a hub resource type pick
       operationId: pickHubResourceType
       tags:

backend/windmill-api/src/apps.rs

@@ -5516,6 +5516,9 @@ mod embed_token_tests {
                 "GET",
             ),
             ("/api/w/test/resources/list_search", "GET"),
+            // The metadata allowlist matches `resources/type/` by prefix, so a route
+            // added under it that is not a read must be denied by its method.
+            ("/api/w/test/resources/type/hub/pick/slack", "POST"),
             // Workspace-wide job enumeration/export must NOT be reachable — an app
             // reads only jobs it launched, by id (blocked via the app_embed sentinel).
             ("/api/w/test/jobs/list", "GET"),

backend/windmill-store/src/resources.rs

@@ -91,7 +91,7 @@ pub fn workspaced_service() -> Router {
         .route("/type/listnames", get(list_resource_types_names))
         .route("/type/resource_counts", get(list_resource_counts_by_type))
         .route("/type/hub/picked", get(list_hub_picked_resource_types))
-        .route("/type/hub/pick/{name}", get(pick_hub_resource_type))
+        .route("/type/hub/pick/{name}", post(pick_hub_resource_type))
         .route("/type/get/{name}", get(get_resource_type))
         .route("/type/exists/{name}", get(exists_resource_type))
         .route("/type/update/{name}", post(update_resource_type))
@@ -2693,10 +2693,17 @@ struct PickHubResourceTypeResult {
 /// Never fails the caller: a hub predating the route, an unreachable one, and a type that
 /// is local-only all mean the same thing — not counted — and the request that reaches here
 /// has already saved the user's resource.
+///
+/// POST, and scoped as a write, because it changes state on the hub under the instance's
+/// own credentials. The sibling `/type/*` routes are metadata reads that a `resources:run`
+/// app-embed token may make, and both the method and this check keep such a token — which
+/// is untrusted app JavaScript — from driving hub counters through us.
 async fn pick_hub_resource_type(
+    authed: ApiAuthed,
     Extension(db): Extension<DB>,
     Path((_w_id, name)): Path<(String, String)>,
 ) -> JsonResult<PickHubResourceTypeResult> {
+    check_scopes(&authed, || "resources:write".to_string())?;
     let hub_base_url = (**windmill_common::HUB_BASE_URL.load()).clone();
     let success = async {
         let id = hub_resource_type_ids(&db, &hub_base_url)
@@ -2736,6 +2743,29 @@ struct HubPickedResourceTypes {
     resource_types: Vec<HubResourceTypePicks>,
 }
 
+#[cfg(test)]
+mod hub_picks_tests {
+    use super::HubPickedResourceTypes;
+
+    /// The hub counts picks in a bigint, which postgres.js serialises as a string. Typing
+    /// the field as a plain i64 fails the whole response, and the ranking silently empties.
+    #[test]
+    fn picks_decode_from_a_string_or_a_number() {
+        let parsed: HubPickedResourceTypes = serde_json::from_str(
+            r#"{"resource_types":[{"name":"slack","picks":"42"},{"name":"github","picks":7}]}"#,
+        )
+        .unwrap();
+        assert_eq!(
+            parsed
+                .resource_types
+                .iter()
+                .map(|rt| (rt.name.as_str(), rt.picks))
+                .collect::<Vec<_>>(),
+            vec![("slack", 42), ("github", 7)]
+        );
+    }
+}
+
 /// The hub's own popularity ranking for resource types. Empty rather than an error when
 /// the hub has no such endpoint, so the pickers reading this treat an older or private hub
 /// as "no hub signal" and fall back to what the workspace itself uses.

frontend/src/lib/components/AppConnectInner.svelte

@@ -42,6 +42,7 @@
 	import { sameTopDomainOrigin } from '$lib/cookies'
 	import SyncResourceTypes from './SyncResourceTypes.svelte'
 	import {
+		alphabetical,
 		byPopularity,
 		hubResourceTypePicks,
 		localResourceTypeCounts,
@@ -390,7 +391,7 @@
 	 * has resources of. Both signals are fetched, so the rows render in the order the lists
 	 * arrived in and re-sort when this lands.
 	 */
-	let popularity: (a: string, b: string) => number = $state(() => 0)
+	let popularity: (a: string, b: string) => number = $state(alphabetical)
 
 	async function loadPopularity() {
 		if (!effectiveWorkspace) return

frontend/src/lib/components/flows/pickers/PickHubScript.svelte

@@ -10,7 +10,11 @@
 	import TextInput from '$lib/components/text_input/TextInput.svelte'
 	import { disableHubStore, workspaceStore } from '$lib/stores'
 	import { logHubScriptPick } from '$lib/utils/featureUsage'
-	import { byPopularity, localResourceTypeCounts } from '$lib/components/pickerPopularity'
+	import {
+		alphabetical,
+		byPopularity,
+		localResourceTypeCounts
+	} from '$lib/components/pickerPopularity'
 
 	interface Props {
 		kind?: HubScriptKind & string
@@ -47,7 +51,7 @@
 	}[] = $state([])
 
 	let allApps: string[] = $state([])
-	let popularity: (a: string, b: string) => number = $state(() => 0)
+	let popularity: (a: string, b: string) => number = $state(alphabetical)
 	let apps: string[] = $derived.by(() =>
 		filter.length > 0
 			? Array.from(new Set(items?.map((x) => x.app) ?? [])).sort(popularity)

frontend/src/lib/components/flows/pickers/PickHubScriptQuick.svelte

@@ -50,7 +50,11 @@
 	import { Alert } from '$lib/components/common'
 	import type { FlowBuilderWhitelabelCustomUi } from '$lib/components/custom_ui'
 	import { logHubScriptPick } from '$lib/utils/featureUsage'
-	import { byPopularity, localResourceTypeCounts } from '$lib/components/pickerPopularity'
+	import {
+		alphabetical,
+		byPopularity,
+		localResourceTypeCounts
+	} from '$lib/components/pickerPopularity'
 
 	let customUi: undefined | FlowBuilderWhitelabelCustomUi = getContext('customUi')
 
@@ -95,7 +99,7 @@
 	}: Props = $props()
 
 	let allApps: string[] = $state([])
-	let popularity: (a: string, b: string) => number = $state(() => 0)
+	let popularity: (a: string, b: string) => number = $state(alphabetical)
 	$effect(() => {
 		if (filter.length > 0) {
 			apps = Array.from(new Set(items?.map((x) => x.app) ?? [])).sort(popularity)

frontend/src/lib/components/pickerPopularity.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from 'vitest'
-import { byPopularity } from './pickerPopularity'
+import { alphabetical, byPopularity } from './pickerPopularity'
 
 const order = (names: string[], hub: Record<string, number>, local: Record<string, number> = {}) =>
 	[...names].sort(byPopularity(hub, local))
@@ -27,4 +27,10 @@ describe('byPopularity', () => {
 			'github'
 		])
 	})
+
+	// The lists render before either signal lands, and one of them arrives in a server-side
+	// HashMap's iteration order, so the resting comparator has to sort rather than no-op.
+	it('leaves an alphabetical order with no signal at all', () => {
+		expect(['stripe', 'ably', 'github'].sort(alphabetical)).toEqual(['ably', 'github', 'stripe'])
+	})
 })

frontend/src/lib/components/pickerPopularity.ts

@@ -1,4 +1,6 @@
+import { get } from 'svelte/store'
 import { ResourceService } from '$lib/gen'
+import { disableHubStore } from '$lib/stores'
 import { createCache } from '$lib/utils'
 import { isCustomResourceTypeName } from './resourceTypeDisplay'
 
@@ -39,8 +41,13 @@ const localCountsCached = createCache(
 	{ invalidateMs: CACHE_MS }
 )
 
-/** What the hub sees people pick, per resource type. Empty on a hub that counts nothing. */
+/**
+ * What the hub sees people pick, per resource type. Empty on a hub that counts nothing,
+ * and on an instance that has switched the hub off — a closed environment must not spend a
+ * request on hub.windmill.dev just to order a list.
+ */
 export function hubResourceTypePicks(workspace: string): Promise<PopularityCounts> {
+	if (get(disableHubStore)) return Promise.resolve({})
 	return hubPicksCached({ workspace })
 }
 
@@ -59,6 +66,7 @@ export function localResourceTypeCounts(workspace: string): Promise<PopularityCo
  * saved a resource. Workspace-made types exist on no hub, so they are not reported.
  */
 export function recordHubResourceTypePick(workspace: string, resourceType: string): void {
+	if (get(disableHubStore)) return
 	if (!resourceType || isCustomResourceTypeName(resourceType)) return
 	ResourceService.pickHubResourceType({ workspace, name: resourceType }).catch(() => {})
 }
@@ -78,3 +86,13 @@ export function byPopularity(
 	return (a, b) =>
 		(hub[b] ?? 0) - (hub[a] ?? 0) || (local[b] ?? 0) - (local[a] ?? 0) || a.localeCompare(b)
 }
+
+/**
+ * The ordering to hold before either signal has landed: the alphabetical floor, which is
+ * what `byPopularity` degrades to anyway.
+ *
+ * A list has to be sorted by *something* from its first paint — one source of these names
+ * is a `HashMap` on the server, so leaving them unsorted means hash order, which differs
+ * between processes.
+ */
+export const alphabetical = byPopularity({}, {})