Add validation to server$

[tzdesign]

What is it about?

server$ can be easily attacked if you don't validate inside or just pretend the data is what typescript tells you

What's the motivation for this proposal?

Problems you are trying to solve:

• Closing an possible attack vector for careless qwik users 😉
• Making sure the parameters are type-safe on both sides
• Adding id to server$ function so that most server$ functions are still reachable if the code changes while deploying it with a lot of nodes (rolling updates)

Goals you are trying to achieve:

• Making qwik more safe and the api even more consistent
• Solving issues with rolling-updates in a Kubernetes cluster

Any other context or information you want to share:

My idea is to add validation in the same way as in actions.

---

Proposed Solution / Feature

What do you propose? (GPT generated 😉 )

The purpose of this initiative is to enhance the safety and consistency of the Qwik framework by addressing potential security risks and improving the reliability of server functions during rolling updates in Kubernetes clusters. This will be achieved by:

1. Implementing type-safe parameters on both the client and server sides to prevent careless usage and potential vulnerabilities.
2. Introducing an ID to the server$ function, ensuring that server-side functions remain accessible and functional during rolling updates.
3. Incorporating validation mechanisms, similar to those used in actions, to strengthen the overall framework.

The ultimate goal is to make Qwik more robust, safe, and reliable in production environments, especially in distributed systems like Kubernetes.

Code examples

// simple
const safeFunction = server$(
      (data) => { … }, 
      zod$(…)
      );

// With ID
const safeFunction = server$(
      (data) => { … }, 
      {id:"safeFunction",validation: [zod$(…)]}
      );

---

Links / References

No response

[wmertens]

I forgot about this one when I wrote #24 😅

The id is useful when moving code around indeed.

[shairez]

Thanks @tzdesign for this proposal! 🙏

We've just reviewed this as a team and here are our thoughts:

We agreed that having a solution for versions skew is important to add and also making sure that server functions are kept safe.

This led us to re-evaluate the problem + solution to make sure we'll end up with an holistic, well thought of solution that will be easier to maintain.

So our suggestion is to split this proposal into 3 potential RFC proposals (as part of an overall solution to the big "server function" problem):

1. Having a generated file (i.e public-api.d.ts) that contains all of the public facing server functions and their type info, that way it could assist with reducing potential unwanted breaking changes to the API.

2. Having a complete solution for long lasting ids and what to do on version skew. We thought about providing a hook for the developers to allow them to decide what to do when there's a mismatch (and also adding "auto-refersh" default behavior if that hook is not being implemented)

3. Validation of params.
   This one was a bit tricky, because unlike server actions, server$ can close over other variables. Plus it also serializes and de-serailizes params, so we would need to see a good reproduction of the problem that needs to be solved here before we continue with the design.
   (Maybe @mhevery could elaborate on the specific example that might do the trick)

So what I suggest:

I'll open 2 other proposals (1 and 2), and this one could be edited to reflect the third one.

WDYT?

[tzdesign]

@shairez
It's great that you looked at this with the team. The validation point in particular is very important for us because we have made some private APIs with server$, which could potentially collect a lot of data from the database. In one example, we enter an array with variant numbers and currently we limit these and protect ourselves with a zod schema within the function body.

Version skew would be important for both actions and also server$, as already described, we have already defined IDs for all actions in order to at least manually output a message as an error instead of getting not found or internal server. A big thing is also empty actions. We have run in weird issues with AWS WAF with empty POSTs. That's why we needed the ids in the first place to get a glimpse of the functions in logs.

I actually hadn't thought about the fact that server$ also serialises. That makes it more complex, but it shouldn't be impossible to validate it at least on server-side. Here is the point where it could get dangerous.

Let me know if you need any more input.

A hook sounds very good to us. We could inform customers for very important functions and then hopefully also actions that the app version is old and they should please perform the action again.

[shairez]

Thanks @tzdesign !

@mhevery would love your take on this, and based on that we'll split this proposal into the 3 mentioned above

[shairez]

OK so we went over this proposal again last night in the RFC meeting.

I went ahead and renamed this to just about the validation and as mentioned above, there are two other proposals for the other steps required for a full solution -
Generated Public API for Server Functions
Call server functions with the manifest hash

We discussed potential way to implement the type safety for closures, this was @mhevery proposal (pseudo code):

let x = 123
let y = "abc"
let server_fn = server$((foo: string, bar: string) => {
    // const [x,y] = useLexicalScope();
    console.log(x, y);
    }), {
    $validator: {
     x: Valibot.number(),
     foo: Valibot.string(),
     // [LINT]: warning missing [y]
    }
})


export server_fn_$_HASH = (a1: string, a2: string) => {
 const [a3,a4] = useLexicalScope();
 console.log(a3, a4);
}
server_fn_$_HASH.$ = "foo,bar|x,y"

The idea is to have the optimizer generate a string containing the names of the params and closed over variables so they could be matched against the Zod / Valibot validation.

[wmertens]

Discussed some more with Fabian.

To keep type inference but also have actual validation, we should make a plugin that maintains a validator on server$.

The validator would be a function that receives the parameters in one array and the scope variables in the second array. It is very important that the scope order matches what the optimizer produces for a qrl.

Then, it should use e.g. ts-morph to discover the types for these, and convert them to valibot assertions.

Then it should change the original code to have this validator.

Furthermore, for complex types, like a brand such as type Foo = string & {T?: 'foo'}, it should complain that it can't check this, and the dev should instead change the types so that they are for example string.

That way, in the server code, the types will be correct but broad enough to validate, and the dev will automatically see where to test further constraints.

E.g. if you pass id: Foo then in the server$ you will have to say id: string for the parameter and check on the server.