mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-13 08:02:47 +00:00
warn when a raw app sql viewer-identity input is not a ctx field (#11704)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
26afeecbd5
commit
36ea2da0bf
@@ -9,6 +9,7 @@ import { GlobalOptions } from "../../types.ts";
|
||||
import { createBundle, detectFrameworks } from "./bundle.ts";
|
||||
import { APP_BACKEND_FOLDER } from "./app_metadata.ts";
|
||||
import { loadRunnablesFromBackend } from "./raw_apps.ts";
|
||||
import { inferSchema } from "../../utils/metadata.ts";
|
||||
import { statedIsolation } from "./app.ts";
|
||||
import {
|
||||
getFolderSuffix,
|
||||
@@ -55,6 +56,50 @@ function validateRawAppYaml(appData: any): {
|
||||
return { errors, warnings };
|
||||
}
|
||||
|
||||
const SQL_LANGUAGES = new Set([
|
||||
"postgresql",
|
||||
"mysql",
|
||||
"bigquery",
|
||||
"snowflake",
|
||||
"mssql",
|
||||
"oracledb",
|
||||
"duckdb",
|
||||
]);
|
||||
|
||||
// The viewer properties a `ctx` field can supply. The Inputs tab of the app
|
||||
// editor hints on the same names (RawAppInlineScriptRunnable.svelte).
|
||||
const VIEWER_IDENTITY_ARGS = ["username", "email", "groups"];
|
||||
|
||||
/**
|
||||
* A SQL runnable argument named after a viewer property is most often a
|
||||
* per-viewer filter. Left as a plain input it takes whatever the browser
|
||||
* sends, so it has to be a `ctx` field for the server to fill it.
|
||||
*/
|
||||
export async function viewerIdentityInputWarnings(
|
||||
runnables: Record<string, any>,
|
||||
): Promise<string[]> {
|
||||
const warnings: string[] = [];
|
||||
for (const [id, runnable] of Object.entries(runnables)) {
|
||||
const { language, content } = runnable?.inlineScript ?? {};
|
||||
if (!SQL_LANGUAGES.has(language) || typeof content !== "string") {
|
||||
continue;
|
||||
}
|
||||
const { schema } = await inferSchema(language, content, {}, id);
|
||||
for (const arg of VIEWER_IDENTITY_ARGS) {
|
||||
const field = runnable.fields?.[arg];
|
||||
const serverFilled = field?.ctx !== undefined ||
|
||||
field?.value !== undefined;
|
||||
if (arg in (schema?.properties ?? {}) && !serverFilled) {
|
||||
warnings.push(
|
||||
`Runnable '${id}': argument '${arg}' is a frontend input, so a viewer can send any value. ` +
|
||||
`If it identifies the viewer, set 'fields: { ${arg}: { ctx: ${arg} } }' in backend/${id}.yaml (next to 'type: inline') so the server fills it.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
return warnings;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that runnables exist either in backend/*.yaml files or in raw_app.yaml
|
||||
*/
|
||||
@@ -84,6 +129,7 @@ async function validateRunnables(
|
||||
" - Or a 'runnables' field in raw_app.yaml (legacy format)",
|
||||
);
|
||||
} else if (hasBackendRunnables) {
|
||||
warnings.push(...await viewerIdentityInputWarnings(runnablesFromBackend));
|
||||
log.info(
|
||||
colors.gray(
|
||||
` Found ${
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import { expect, test } from "bun:test";
|
||||
// app.ts first: it is the entry the CLI loads lint.ts through, and loading
|
||||
// lint.ts ahead of it trips their import cycle.
|
||||
import "../src/commands/app/app.ts";
|
||||
import { viewerIdentityInputWarnings } from "../src/commands/app/lint.ts";
|
||||
|
||||
const sql = (content: string, fields?: Record<string, any>) => ({
|
||||
type: "inline",
|
||||
inlineScript: { language: "duckdb", content },
|
||||
...(fields ? { fields } : {}),
|
||||
});
|
||||
|
||||
test("app lint warns on a SQL viewer-identity argument the server does not fill", async () => {
|
||||
const content =
|
||||
"-- $username (text)\n-- $limit (int)\nSELECT * FROM orders WHERE owner = $username LIMIT $limit";
|
||||
|
||||
const warnings = await viewerIdentityInputWarnings({
|
||||
open: sql(content),
|
||||
filled: sql(content, { username: { ctx: "username" } }),
|
||||
});
|
||||
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toContain("Runnable 'open': argument 'username'");
|
||||
});
|
||||
@@ -15,6 +15,7 @@
|
||||
import { Pane, Splitpanes } from 'svelte-splitpanes'
|
||||
import Tabs from '../common/tabs/Tabs.svelte'
|
||||
import { Tab } from '../common'
|
||||
import { SQL_LANGUAGES } from '$lib/infer'
|
||||
import RawAppInputsSpecEditor from './RawAppInputsSpecEditor.svelte'
|
||||
import SplitPanesWrapper from '../splitPanes/SplitPanesWrapper.svelte'
|
||||
import SchemaForm from '../SchemaForm.svelte'
|
||||
@@ -80,6 +81,13 @@
|
||||
}
|
||||
}
|
||||
|
||||
// A SQL argument named after a viewer property is most often a per-viewer filter,
|
||||
// which only a ctx field protects. `wmill app lint` warns on the same names.
|
||||
const VIEWER_IDENTITY_ARGS = ['username', 'email', 'groups']
|
||||
let isSql = $derived(
|
||||
isRunnableByName(runnable) && SQL_LANGUAGES.includes(runnable.inlineScript?.language ?? '')
|
||||
)
|
||||
|
||||
let selectedTab = $state('test')
|
||||
let args = $state({})
|
||||
let hubFlowPreview: OpenFlow | undefined = $state(undefined)
|
||||
@@ -266,6 +274,7 @@
|
||||
documentationLink={meta?.['documentationLink']}
|
||||
allowTypeChange={meta?.['allowTypeChange']}
|
||||
displayType
|
||||
suggestCtx={isSql && VIEWER_IDENTITY_ARGS.includes(k)}
|
||||
/>
|
||||
{/each}
|
||||
</div>
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
import { fieldTypeToTsType } from '../apps/utils'
|
||||
import type { InputType } from '../apps/inputType'
|
||||
import Select from '$lib/components/select/Select.svelte'
|
||||
import { Alert } from '$lib/components/common'
|
||||
import { userStore } from '$lib/stores'
|
||||
import {
|
||||
useOperatingUser,
|
||||
@@ -67,6 +68,8 @@
|
||||
allowTypeChange?: boolean
|
||||
loading?: boolean
|
||||
documentationLink?: string | undefined
|
||||
/** Warn that this input, left as a frontend input, is open to any value a viewer sends. */
|
||||
suggestCtx?: boolean
|
||||
}
|
||||
|
||||
let {
|
||||
@@ -85,7 +88,8 @@
|
||||
displayType = false,
|
||||
allowTypeChange = true,
|
||||
loading = false,
|
||||
documentationLink = undefined
|
||||
documentationLink = undefined,
|
||||
suggestCtx = false
|
||||
}: Props = $props()
|
||||
|
||||
run(() => {
|
||||
@@ -193,6 +197,11 @@
|
||||
</div>
|
||||
{:else if componentInput?.type === 'user' || componentInput?.type == undefined}
|
||||
<span class="text-2xs italic text-primary">Field's value is a frontend input</span>
|
||||
{#if suggestCtx}
|
||||
<Alert type="warning" size="xs" title="A viewer can send any value">
|
||||
If this identifies the viewer, switch it to context so the server fills it.
|
||||
</Alert>
|
||||
{/if}
|
||||
{/if}
|
||||
{#if componentInput?.type === 'user' && ((fieldType == 'object' && format?.startsWith('resource-') && format !== 'resource-s3_object') || fieldType == 'resource')}
|
||||
<div class="flex flex-row items-center">
|
||||
|
||||
@@ -301,7 +301,7 @@ export async function inferAnsibleExecutionMode(code: string): Promise<any> {
|
||||
}
|
||||
}
|
||||
|
||||
const SQL_LANGUAGES = [
|
||||
export const SQL_LANGUAGES = [
|
||||
'postgresql',
|
||||
'mysql',
|
||||
'bigquery',
|
||||
|
||||
Reference in New Issue
Block a user