From 7bac376599571f66ba5f6ca265de5404a3a40640 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 17 Feb 2025 09:34:34 +0100 Subject: [PATCH 1/7] Initial js validation --- .../common/CodeEditor/CodeEditor.svelte | 4 + .../common/CodeEditor/validator/js.ts | 80 +++++++++++++++++++ .../common/bindings/BindingPanel.svelte | 1 + 3 files changed, 85 insertions(+) create mode 100644 packages/builder/src/components/common/CodeEditor/validator/js.ts diff --git a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte index fbf74d1e9b..0f4bc64e2a 100644 --- a/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte +++ b/packages/builder/src/components/common/CodeEditor/CodeEditor.svelte @@ -49,6 +49,7 @@ import type { EditorMode } from "@budibase/types" import type { BindingCompletion, CodeValidator } from "@/types" import { validateHbsTemplate } from "./validator/hbs" + import { validateJsTemplate } from "./validator/js" export let label: string | undefined = undefined export let completions: BindingCompletion[] = [] @@ -356,6 +357,9 @@ if (mode === EditorModes.Handlebars) { const diagnostics = validateHbsTemplate(value, validations) editor.dispatch(setDiagnostics(editor.state, diagnostics)) + } else if (mode === EditorModes.JS) { + const diagnostics = validateJsTemplate(value, validations) + editor.dispatch(setDiagnostics(editor.state, diagnostics)) } } diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts new file mode 100644 index 0000000000..6e85dc41ec --- /dev/null +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -0,0 +1,80 @@ +import { Parser } from "acorn" +import { simple as walk } from "acorn-walk" + +import { iifeWrapper } from "@budibase/string-templates" +import type { Diagnostic } from "@codemirror/lint" +import { CodeValidator } from "@/types" + +export function validateJsTemplate( + code: string, + validations: CodeValidator +): Diagnostic[] { + const diagnostics: Diagnostic[] = [] + + try { + // const helperUsages = new RegExp(/\bhelpers\.(\w)+\b/).exec(code) + const ast = Parser.parse(iifeWrapper(code), { + ecmaVersion: "latest", + locations: true, + }) + + const lineOffsets: number[] = [0] + let offset = 0 + for (const line of code.split("\n")) { + lineOffsets.push(offset) + offset += line.length + 1 // +1 for newline character + } + + walk(ast, { + CallExpression(node) { + const callee: any = node.callee + if ( + node.type === "CallExpression" && + callee.object?.name === "helpers" && + node.loc + ) { + const functionName = callee.property.name + const from = + lineOffsets[node.loc.start.line - 1] + node.loc.start.column + const to = lineOffsets[node.loc.end.line - 1] + node.loc.end.column + + if (!(functionName in validations)) { + diagnostics.push({ + from, + to, + severity: "warning", + message: `"${functionName}" function does not exist.`, + }) + return + } + + const { arguments: expectedArguments } = validations[functionName] + if ( + expectedArguments && + node.arguments.length !== expectedArguments.length + ) { + diagnostics.push({ + from, + to, + severity: "error", + message: `Function "${functionName}" expects ${ + expectedArguments.length + } parameters (${expectedArguments.join(", ")}), but got ${ + node.arguments.length + }.`, + }) + } + } + }, + }) + } catch (e: any) { + diagnostics.push({ + from: 0, + to: code.length, + severity: "error", + message: `Syntax error: ${e.message}`, + }) + } + + return diagnostics +} diff --git a/packages/builder/src/components/common/bindings/BindingPanel.svelte b/packages/builder/src/components/common/bindings/BindingPanel.svelte index 2c35acdf2d..9ebbc91309 100644 --- a/packages/builder/src/components/common/bindings/BindingPanel.svelte +++ b/packages/builder/src/components/common/bindings/BindingPanel.svelte @@ -377,6 +377,7 @@ value={jsValue ? decodeJSBinding(jsValue) : jsValue} on:change={onChangeJSValue} {completions} + {validations} mode={EditorModes.JS} bind:getCaretPosition bind:insertAtPos From 608fcb42be494dbf418fff23fc4eb75bfb2ca9de Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:24:19 +0100 Subject: [PATCH 2/7] Validate return value --- .../components/common/CodeEditor/validator/js.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts index 6e85dc41ec..6d7c3ea8bf 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/js.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -25,7 +25,11 @@ export function validateJsTemplate( offset += line.length + 1 // +1 for newline character } + let hasReturnStatement = false walk(ast, { + ReturnStatement(node) { + hasReturnStatement = !!node.argument + }, CallExpression(node) { const callee: any = node.callee if ( @@ -67,6 +71,15 @@ export function validateJsTemplate( } }, }) + + if (!hasReturnStatement) { + diagnostics.push({ + from: 0, + to: code.length, + severity: "error", + message: "Your code must return a value.", + }) + } } catch (e: any) { diagnostics.push({ from: 0, From c5f2ef9354917c930cab6aef42470b2bfdcad7c8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:35:52 +0100 Subject: [PATCH 3/7] Add js validation tests --- .../CodeEditor/validator/tests/js.spec.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts new file mode 100644 index 0000000000..be6977f35a --- /dev/null +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -0,0 +1,71 @@ +import { validateJsTemplate } from "../js" +import { CodeValidator } from "@/types" + +describe("js validator", () => { + it("validates valid code", () => { + const text = "return 7" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("does not validate runtime errors", () => { + const text = "return a" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("validates multiline code", () => { + const text = "const foo='bar'\nreturn 123" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + describe("helpers", () => { + const validators: CodeValidator = { + helperFunction: { + arguments: ["a", "b", "c"], + }, + } + + it("validates helpers with valid params", () => { + const text = "return helpers.helperFunction(1, 99, 'a')" + + const result = validateJsTemplate(text, validators) + expect(result).toHaveLength(0) + }) + + it("throws on too few params", () => { + const text = "return helpers.helperFunction(100)" + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 7, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 1.`, + severity: "error", + to: 34, + }, + ]) + }) + + it("throws on too many params", () => { + const text = "return helpers.helperFunction( 1, 99, 'a', 100)" + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 7, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 4.`, + severity: "error", + to: 47, + }, + ]) + }) + }) +}) From fba5663e66aeb34605529896e7bb085e34a4d5b9 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 12:44:47 +0100 Subject: [PATCH 4/7] Improve tests --- .../CodeEditor/validator/tests/js.spec.ts | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts index be6977f35a..d0c25182f7 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -26,6 +26,29 @@ describe("js validator", () => { expect(result).toEqual([]) }) + it("allows return not being on the last line", () => { + const text = "const foo='bar'\nreturn 123\nconsole.log(foo)" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([]) + }) + + it("throws on missing return", () => { + const text = "const foo='bar'\nbar='foo'" + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 0, + message: "Your code must return a value.", + severity: "error", + to: 25, + }, + ]) + }) + describe("helpers", () => { const validators: CodeValidator = { helperFunction: { @@ -37,7 +60,7 @@ describe("js validator", () => { const text = "return helpers.helperFunction(1, 99, 'a')" const result = validateJsTemplate(text, validators) - expect(result).toHaveLength(0) + expect(result).toEqual([]) }) it("throws on too few params", () => { @@ -67,5 +90,49 @@ describe("js validator", () => { }, ]) }) + + it("validates helpers on inner functions", () => { + const text = `function call(){ + return helpers.helperFunction(1, 99) + } + return call()` + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 46, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 2.`, + severity: "error", + to: 75, + }, + ]) + }) + + it("validates multiple helpers", () => { + const text = + "return helpers.helperFunction(1, 99, 'a') + helpers.helperFunction(1) + helpers.another(1) + helpers.another()" + const validators: CodeValidator = { + helperFunction: { + arguments: ["a", "b", "c"], + }, + another: { arguments: [] }, + } + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 44, + message: `Function "helperFunction" expects 3 parameters (a, b, c), but got 1.`, + severity: "error", + to: 69, + }, + { + from: 72, + message: `Function "another" expects 0 parameters (), but got 1.`, + severity: "error", + to: 90, + }, + ]) + }) }) }) From 1a4abb7630b5c72ac18e9c5267e2f583caa28a99 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 19 Feb 2025 13:14:40 +0100 Subject: [PATCH 5/7] Validate return --- .../common/CodeEditor/validator/js.ts | 24 ++++++++++++------- .../CodeEditor/validator/tests/js.spec.ts | 18 ++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/packages/builder/src/components/common/CodeEditor/validator/js.ts b/packages/builder/src/components/common/CodeEditor/validator/js.ts index 6d7c3ea8bf..20fb5abd07 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/js.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/js.ts @@ -1,7 +1,6 @@ import { Parser } from "acorn" -import { simple as walk } from "acorn-walk" +import * as walk from "acorn-walk" -import { iifeWrapper } from "@budibase/string-templates" import type { Diagnostic } from "@codemirror/lint" import { CodeValidator } from "@/types" @@ -12,13 +11,13 @@ export function validateJsTemplate( const diagnostics: Diagnostic[] = [] try { - // const helperUsages = new RegExp(/\bhelpers\.(\w)+\b/).exec(code) - const ast = Parser.parse(iifeWrapper(code), { + const ast = Parser.parse(code, { ecmaVersion: "latest", locations: true, + allowReturnOutsideFunction: true, }) - const lineOffsets: number[] = [0] + const lineOffsets: number[] = [] let offset = 0 for (const line of code.split("\n")) { lineOffsets.push(offset) @@ -26,9 +25,18 @@ export function validateJsTemplate( } let hasReturnStatement = false - walk(ast, { - ReturnStatement(node) { - hasReturnStatement = !!node.argument + walk.ancestor(ast, { + ReturnStatement(node, _state, ancestors) { + if ( + // it returns a value + node.argument && + // and it is top level + ancestors.length === 2 && + ancestors[0].type === "Program" && + ancestors[1].type === "ReturnStatement" + ) { + hasReturnStatement = true + } }, CallExpression(node) { const callee: any = node.callee diff --git a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts index d0c25182f7..cd9fe4684c 100644 --- a/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts +++ b/packages/builder/src/components/common/CodeEditor/validator/tests/js.spec.ts @@ -49,6 +49,24 @@ describe("js validator", () => { ]) }) + it("checks that returns are at top level", () => { + const text = ` + function call(){ + return 1 + }` + const validators = {} + + const result = validateJsTemplate(text, validators) + expect(result).toEqual([ + { + from: 0, + message: "Your code must return a value.", + severity: "error", + to: text.length, + }, + ]) + }) + describe("helpers", () => { const validators: CodeValidator = { helperFunction: { From de62913afdf96e7c8c02d8c2c16b8f037cca0966 Mon Sep 17 00:00:00 2001 From: jvcalderon Date: Thu, 20 Feb 2025 23:16:58 +0100 Subject: [PATCH 6/7] [Revert] store koa sessions in redis instead of cookies --- hosting/nginx.dev.conf | 6 ---- packages/worker/package.json | 1 - .../src/api/routes/global/tests/auth.spec.ts | 2 +- packages/worker/src/index.ts | 26 ++--------------- packages/worker/src/koa-redis.d.ts | 1 - yarn.lock | 28 ++----------------- 6 files changed, 5 insertions(+), 59 deletions(-) delete mode 100644 packages/worker/src/koa-redis.d.ts diff --git a/hosting/nginx.dev.conf b/hosting/nginx.dev.conf index a8cefe9ccc..747235e8ef 100644 --- a/hosting/nginx.dev.conf +++ b/hosting/nginx.dev.conf @@ -62,12 +62,6 @@ http { proxy_connect_timeout 120s; proxy_send_timeout 120s; proxy_http_version 1.1; - - # Enable buffering for potentially large OIDC configs - proxy_buffering on; - proxy_buffer_size 16k; - proxy_buffers 4 32k; - proxy_set_header Host $host; proxy_set_header Connection ""; diff --git a/packages/worker/package.json b/packages/worker/package.json index 28728272ca..53d14dacee 100644 --- a/packages/worker/package.json +++ b/packages/worker/package.json @@ -62,7 +62,6 @@ "koa-body": "4.2.0", "koa-compress": "4.0.1", "koa-passport": "4.1.4", - "koa-redis": "^4.0.1", "koa-send": "5.0.1", "koa-session": "5.13.1", "koa-static": "5.0.0", diff --git a/packages/worker/src/api/routes/global/tests/auth.spec.ts b/packages/worker/src/api/routes/global/tests/auth.spec.ts index f89cb4a027..bff959469e 100644 --- a/packages/worker/src/api/routes/global/tests/auth.spec.ts +++ b/packages/worker/src/api/routes/global/tests/auth.spec.ts @@ -311,7 +311,7 @@ describe("/api/global/auth", () => { }) }) - describe.skip("GET /api/global/auth/:tenantId/oidc/callback", () => { + describe("GET /api/global/auth/:tenantId/oidc/callback", () => { it("logs in", async () => { const email = `${generator.guid()}@example.com` diff --git a/packages/worker/src/index.ts b/packages/worker/src/index.ts index f382aa8a20..0547afab38 100644 --- a/packages/worker/src/index.ts +++ b/packages/worker/src/index.ts @@ -4,7 +4,7 @@ if (process.env.DD_APM_ENABLED) { // need to load environment first import env from "./environment" -import Application, { Middleware } from "koa" +import Application from "koa" import { bootstrap } from "global-agent" import * as db from "./db" import { sdk as proSdk } from "@budibase/pro" @@ -20,7 +20,6 @@ import { cache, features, } from "@budibase/backend-core" -import RedisStore from "koa-redis" db.init() import koaBody from "koa-body" @@ -53,28 +52,7 @@ app.proxy = true app.use(handleScimBody) app.use(koaBody({ multipart: true })) -const sessionMiddleware: Middleware = async (ctx: any, next: any) => { - const redisClient = await new redis.Client( - redis.utils.Databases.SESSIONS - ).init() - return koaSession( - { - // @ts-ignore - store: new RedisStore({ client: redisClient.getClient() }), - key: "koa:sess", - maxAge: 86400000, // one day - httpOnly: true, - secure: process.env.NODE_ENV === "production", - sameSite: "strict", - rolling: true, - renew: true, - }, - app - )(ctx, next) -} - -app.use(sessionMiddleware) - +app.use(koaSession(app)) app.use(middleware.correlation) app.use(middleware.pino) app.use(middleware.ip) diff --git a/packages/worker/src/koa-redis.d.ts b/packages/worker/src/koa-redis.d.ts deleted file mode 100644 index ad1b7a46f1..0000000000 --- a/packages/worker/src/koa-redis.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare module "koa-redis" {} diff --git a/yarn.lock b/yarn.lock index 8f611e224c..ceae41458c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2695,13 +2695,6 @@ dependencies: regenerator-runtime "^0.14.0" -"@babel/runtime@^7.8.3": - version "7.26.9" - resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.26.9.tgz#aa4c6facc65b9cb3f87d75125ffd47781b475433" - integrity sha512-aA63XwOkcl4xxQa3HjPMqOP6LiK0ZDv3mUPYEFXkpHbaFjtGggE1A61FjFzJnB+p7/oy2gA8E+rcBNl/zC1tMg== - dependencies: - regenerator-runtime "^0.14.0" - "@babel/template@^7.22.15", "@babel/template@^7.22.5", "@babel/template@^7.25.9", "@babel/template@^7.3.3": version "7.25.9" resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.25.9.tgz#ecb62d81a8a6f5dc5fe8abfc3901fc52ddf15016" @@ -9048,14 +9041,7 @@ co-body@^5.1.1: raw-body "^2.2.0" type-is "^1.6.14" -co-wrap-all@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/co-wrap-all/-/co-wrap-all-1.0.0.tgz#370ae3e8333510a53f6b2f7fdfbe4568a11b7ecf" - integrity sha512-aru6gLi2vTUazr+MxVm3Rv6ST7/EKtFj9BrfkcOrbCO2Qv6LqJdE71m88HhHiBEviKw/ucVrwoGLrq2xHpOsJA== - dependencies: - co "^4.0.0" - -co@^4.0.0, co@^4.6.0: +co@^4.6.0: version "4.6.0" resolved "https://registry.yarnpkg.com/co/-/co-4.6.0.tgz#6ea6bdf3d853ae54ccb8e47bfa0bf3f9031fb184" integrity sha512-QVb0dM5HvG+uaxitm8wONl7jltx8dqhfU33DcqtOZcLSVIKSDDLDi7+0LbAKiyI8hD9u42m2YxXSkMGWThaecQ== @@ -13191,7 +13177,7 @@ ioredis@5.3.2: redis-parser "^3.0.0" standard-as-callback "^2.1.0" -ioredis@^4.14.1, ioredis@^4.28.5: +ioredis@^4.28.5: version "4.28.5" resolved "https://registry.yarnpkg.com/ioredis/-/ioredis-4.28.5.tgz#5c149e6a8d76a7f8fa8a504ffc85b7d5b6797f9f" integrity sha512-3GYo0GJtLqgNXj4YhrisLaNNvWSNwSS2wS4OELGfGxH8I69+XfNdnmV1AyN+ZqMh0i7eX+SWjrwFKDBDgfBC1A== @@ -14691,16 +14677,6 @@ koa-pino-logger@4.0.0: dependencies: pino-http "^6.5.0" -koa-redis@^4.0.1: - version "4.0.1" - resolved "https://registry.yarnpkg.com/koa-redis/-/koa-redis-4.0.1.tgz#57ac1b46d9ab851221a9f4952c1e8d4bf289db40" - integrity sha512-o2eTVNo1NBnloeUGhHed5Q2ZvJSLpUEj/+E1/7oH5EmH8WuQ+QLdl/VawkshxdFQ47W1p6V09lM3hCTu7D0YnQ== - dependencies: - "@babel/runtime" "^7.8.3" - co-wrap-all "^1.0.0" - debug "^4.1.1" - ioredis "^4.14.1" - koa-router@^10.0.0: version "10.1.1" resolved "https://registry.yarnpkg.com/koa-router/-/koa-router-10.1.1.tgz#20809f82648518b84726cd445037813cd99f17ff" From add89b87d230d755b949cc4d53cb2f6227777acf Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Fri, 21 Feb 2025 14:02:45 +0000 Subject: [PATCH 7/7] Bump version to 3.4.16 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 91980e0a15..bb71d10f41 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "3.4.15", + "version": "3.4.16", "npmClient": "yarn", "concurrency": 20, "command": {