feat: Hasura input validation for tables storing user-submitted HTML #2551

DafyddLlyr · 2023-12-11T19:29:58Z

What does this PR do?

Attempts to resolve the issue of sanitation of user-submitted HTML in a generalised method using Hasura input validation
Adds a new API endpoint to handle input validation
Adds validation to global_settings and flows tables (this is not available on a per-column basis)
Adds very basic frontend feedback (a toast message)

Apologies for such a big PR... 🙏

How does this work?

Essentially, prior to opening a database transaction, Hasura makes a request to an endpoint which can run business logic to validate a payload.

This endpoint does not modify the submitted payload, it merely accepts or rejects it. This is different to the previous approach of sanitising and accepting changes.

Please see diagram below which demonstrates the data flow -

sequenceDiagram
  participant PlanX as PlanX Editor
  participant Hasura as Hasura GraphQL API
  participant API as PlanX API
  participant Middleware as Validation Middleware
  participant Controller as Controller
  participant DB as PostgreSQL

  PlanX ->> Hasura: User-submitted content
  activate Hasura

  Hasura ->> API: Input validation request
  activate API

  activate API
  API ->> Middleware: Validate against Zod schema
  activate Middleware
  alt Valid
    Middleware ->> Controller: Transformed payload
    activate Controller
    Controller ->> Hasura: 200 OK
    deactivate Controller
    Hasura ->> DB: Write to Postgres
    activate DB
    DB ->> Hasura: Success
    deactivate DB
    Hasura ->> PlanX: 200 OK (Empty)
  else Invalid
    Middleware ->> Controller: Transformed payload
    deactivate Middleware
    activate Controller
    Controller ->> Hasura: 400 Bad Request
    deactivate Controller
  deactivate API
    Hasura ->> PlanX: 400 Bad Request (Invalid HTML content)
  end

  deactivate Hasura

Why input validation? Why not Hasura actions?

This approach is fairly re-usable and generalisable so it can be used, re-used, and expanded upon.

Our initial crack at this in #2488 is pretty specific to implementation and has a few gotchas / edge cases / footguns around permissions which are covered by comments etc. If we needed to do this for multiple tables things could get quite messy.

This approach means there's a single bit of logic for user-submitted HTML which can be used by multiple tables.

The real catch here is that it's just validation - it doesn't sanitise the input and proceed. Realistically this is likely fine for unsafe HTML!

I had initially considered Hasura Actions as a viable method as this could handle transforming data and then writing to the database but I ultimately decided against it as -

It would require some custom code per-table
We don't actually need sanitised HTML in the DB, rejecting it is fine
It just seemed more complex all around

Testing

Screen.Recording.2023-12-13.at.08.53.34.mov

Updates to global_settings works for clean content, is rejected for unsafe content
Updates to flows works for clean content, is rejected for unsafe content (flow.data still updates via ShareDB)
The same validation check happens via the GraphQL API directly

Next steps...

A basic E2E test or two (another PR)
Revert previous implementation in fix: Restrict write access to flows.data column via GraphQL API #2488 (another PR)
Merge chore: Update Hasura to v2.36.0 #2549 and rebase to remove diff noise in tables.yaml

github-actions · 2023-12-11T19:30:54Z

🤖 Hasura Change Summary compared a subset of table metadata including permissions:

Updated Tables (2)

public.flows permissions:

	insert	update
api	^➕/_➖	^➕/_➖
platformAdmin	^➕/_➖	^➕/_➖
teamEditor	^➕/_➖	^➕/_➖

public.global_settings permissions:

	insert	select	update	delete
platformAdmin	^➕/_➖		^➕/_➖

github-actions · 2023-12-11T19:53:51Z

Removed vultr server and associated DNS entries

DafyddLlyr · 2023-12-12T17:04:44Z

api.planx.uk/modules/webhooks/routes.ts

@@ -53,6 +55,12 @@ router.post(
  sanitiseApplicationDataController,
 );

+router.post(
+  "/webhooks/hasura/validate-input/jsonb/clean-html",


Open to thoughts on the URL path here - I figured having a column type parameter would handle us possibly expanding this in future, e.g -

/webhooks/hasura/validate-input/text/clean-html

/webhooks/hasura/validate-input/number/is-uprn

DafyddLlyr · 2023-12-12T17:05:49Z

api.planx.uk/modules/webhooks/service/validateInput/utils.test.ts

+});
+
+describe("isCleanHTML() helper function", () => {
+  const dirtyHTML = [


Examples taken from https://github.com/cure53/DOMPurify?tab=readme-ov-file#some-purification-samples-please

DafyddLlyr · 2023-12-12T17:08:50Z

api.planx.uk/modules/webhooks/service/validateInput/utils.ts

+ * Recursively iterate over an object, checking is validator callback condition is met
+ * Fails fast - will return false once first invalid value is encountered
+ */
+export const isObjectValid = (


I was initially hoping to just use some lodash method to get all nested values from an object so I didn't have to write this (and then write tests for it!).

I don't think there is a method for this however - a few requests on the lodash repo suggest using flat but a whole library for this seems a bit excessive.

If there's a simpler library method or approach for this that I'm just missing please let me know 👍

This still feels readable to me, agree a whole library feels like overkill at this stage!

DafyddLlyr · 2023-12-12T17:29:27Z

api.planx.uk/modules/webhooks/service/validateInput/schema.ts

+type isCleanJSONBSchema = z.ZodType<
+  IsCleanJSONBRequest,
+  z.ZodTypeDef,
+  HasuraValidateInputRequest
+>;


Using .transform() to join two schemas requires this pretty gnarly type.

Docs: https://zod.dev/?id=zodtype-with-zodeffects

DafyddLlyr · 2023-12-12T18:25:35Z

hasura.planx.uk/metadata/tables.yaml

@@ -290,6 +290,15 @@
          - id
          - version
          - data
+        validate_input:


These are repeated per table, per role, per operation (insert & update).

Now that we've updated Hasura it might be worth re-looking at inherited roles as a way of simplifying a few things.

jessicamcinchak

This all looks good to me - really nice to see immediate use cases for these new features like input validation 🎉

jessicamcinchak · 2023-12-15T12:33:24Z

api.planx.uk/modules/webhooks/service/validateInput/utils.test.ts

+    `<p>This is <b>bold</b> text and <i>italic</i> text.</p>`,
+    `<div><p>This content is inside a div.</p></div>`,
+    `<h2>Subheading</h2><p>This is a paragraph under the subheading.</p>`,
+    `<div><h1>Main Title</h1><p>This is a <b>nested</b> paragraph with a <i>nested</i> element.</p></div>`,


Based on what we know about Planx content / rich text editing - should we have a clean example with a valid <img> tag too just to be safe?

This is a really good shout - test case added 👍

jessicamcinchak · 2023-12-15T12:34:46Z

api.planx.uk/modules/webhooks/service/validateInput/utils.ts

+ * Recursively iterate over an object, checking is validator callback condition is met
+ * Fails fast - will return false once first invalid value is encountered
+ */
+export const isObjectValid = (


This still feels readable to me, agree a whole library feels like overkill at this stage!

DafyddLlyr force-pushed the dp/postgres-input-validation-sanitation branch from 4876ee1 to 812165d Compare December 11, 2023 19:33

DafyddLlyr commented Dec 12, 2023

View reviewed changes

DafyddLlyr force-pushed the dp/postgres-input-validation-sanitation branch from c20cd16 to c3d809c Compare December 12, 2023 17:28

DafyddLlyr commented Dec 12, 2023

View reviewed changes

DafyddLlyr added 8 commits December 12, 2023 18:23

feat: Setup Hasura input validation on global_settings and flow tables

3696df5

feat(wip): Setup utils for validation

9411041

chore: Update validator types

84cdff0

feat: Setup validation schema and controller

f617704

docs: Swagger docs

7c88bde

fix: Linting

d914ebe

feat: Frontend error handling

166d2bf

chore: Clean up types

b806764

DafyddLlyr force-pushed the dp/postgres-input-validation-sanitation branch from c3d809c to b806764 Compare December 12, 2023 18:23

DafyddLlyr commented Dec 12, 2023

View reviewed changes

DafyddLlyr requested a review from a team December 12, 2023 18:26

DafyddLlyr marked this pull request as ready for review December 12, 2023 18:26

DafyddLlyr changed the title ~~feat(wip): Hasura input validation for tables storing user-submitted HTML~~ feat: Hasura input validation for tables storing user-submitted HTML Dec 12, 2023

jessicamcinchak approved these changes Dec 15, 2023

View reviewed changes

test: Add test case with img tag

62623ab

DafyddLlyr merged commit fc08523 into main Dec 18, 2023
12 checks passed

DafyddLlyr deleted the dp/postgres-input-validation-sanitation branch December 18, 2023 13:24

This was referenced Dec 21, 2023

fix: Drop redundant sanitation for Find and Replace #2598

Merged

chore: Restore permissions for flow.data column #2599

Merged

feat: Drop permission workaround for copy flow #2600

Merged

DafyddLlyr mentioned this pull request Jan 4, 2024

fix: Unescape HTML entities for alternate DOMPurify check #2630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Hasura input validation for tables storing user-submitted HTML #2551

feat: Hasura input validation for tables storing user-submitted HTML #2551

DafyddLlyr commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading

DafyddLlyr Dec 12, 2023 •

edited

Loading

DafyddLlyr Dec 12, 2023

DafyddLlyr Dec 12, 2023 •

edited

Loading

jessicamcinchak Dec 15, 2023

DafyddLlyr Dec 12, 2023

DafyddLlyr Dec 12, 2023 •

edited

Loading

jessicamcinchak left a comment

jessicamcinchak Dec 15, 2023

DafyddLlyr Dec 18, 2023

jessicamcinchak Dec 15, 2023

feat: Hasura input validation for tables storing user-submitted HTML #2551

feat: Hasura input validation for tables storing user-submitted HTML #2551

Conversation

DafyddLlyr commented Dec 11, 2023 • edited Loading

What does this PR do?

How does this work?

Why input validation? Why not Hasura actions?

Testing

Next steps...

github-actions bot commented Dec 11, 2023 • edited Loading

Updated Tables (2)

github-actions bot commented Dec 11, 2023 • edited Loading

DafyddLlyr Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

DafyddLlyr Dec 12, 2023

Choose a reason for hiding this comment

DafyddLlyr Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

jessicamcinchak Dec 15, 2023

Choose a reason for hiding this comment

DafyddLlyr Dec 12, 2023

Choose a reason for hiding this comment

DafyddLlyr Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

jessicamcinchak left a comment

Choose a reason for hiding this comment

jessicamcinchak Dec 15, 2023

Choose a reason for hiding this comment

DafyddLlyr Dec 18, 2023

Choose a reason for hiding this comment

jessicamcinchak Dec 15, 2023

Choose a reason for hiding this comment

DafyddLlyr commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading

github-actions bot commented Dec 11, 2023 •

edited

Loading

DafyddLlyr Dec 12, 2023 •

edited

Loading

DafyddLlyr Dec 12, 2023 •

edited

Loading

DafyddLlyr Dec 12, 2023 •

edited

Loading