Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: RN-1112: bump knex to 3.1.0 #5483

Merged
merged 16 commits into from
Apr 4, 2024
Merged

deps: RN-1112: bump knex to 3.1.0 #5483

merged 16 commits into from
Apr 4, 2024

Conversation

jaskfla
Copy link
Contributor

@jaskfla jaskfla commented Mar 8, 2024

Issue RN-1112: Bump knex to 3.1.0

Changes

  • This PR supersedes the Bump knex from 2.3.0 to 2.4.0 #4297 Dependabot PR, which addresses a critical severity involving SQL injection
  • 2.4.0 is the version that resolves the vulnerability which Dependabot alerted us to, but figured this was a good opportunity to bump to the latest version
  • This necessitates one workaround: @tupaia/database’s getColSelector() currently produces -> and ->> JSON selectors in its output, but for some reason this breaks with newer versions of Knex where positional parameters get out of order and messes up queries. I’ve fixed this by making getColSelector() construct equivalent queries using #>>, but usage of the function remains the same. It should continue to be provided input strings with -> and ->>.

@jaskfla jaskfla marked this pull request as ready for review March 8, 2024 03:47
@jaskfla jaskfla changed the title bump knex to 3.1.0 RN-1112: Bump knex to 3.1.0 Mar 8, 2024
@jaskfla jaskfla added the dependencies Pull requests that update a dependency file label Mar 14, 2024
@jaskfla jaskfla requested a review from rohan-bes March 14, 2024 03:21
Copy link
Collaborator

@rohan-bes rohan-bes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be alright for me to ask you to look into exactly what's going wrong with the failing test in the @tupaia/database package when we upgrade it?

It looks like once again it's related to jsonb operators (we use them in the dashboard_relation.entity_attributes_filter functionality). If it's possible for us to find out how to get that working on the latest version of knex I'd love if we could upgrade globally 👍

@jaskfla jaskfla requested a review from rohan-bes April 2, 2024 01:31
@jaskfla jaskfla changed the title RN-1112: Bump knex to 3.1.0 deps: RN-1112: bump knex to 3.1.0 Apr 2, 2024
yarn.lock Outdated Show resolved Hide resolved
packages/database/src/TupaiaDatabase.js Outdated Show resolved Hide resolved
jaskfla and others added 2 commits April 2, 2024 15:45
Co-authored-by: Rohan Port <59544282+rohan-bes@users.noreply.github.com>
@jaskfla jaskfla requested a review from rohan-bes April 2, 2024 02:53
@jaskfla jaskfla requested a review from rohan-bes April 4, 2024 04:21
Copy link
Collaborator

@rohan-bes rohan-bes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks for persevering through this 💪

You might need to rebase to get the github actions to pass?

packages/database/src/TupaiaDatabase.js Show resolved Hide resolved
# Conflicts:
#	packages/data-lake-api/package.json
#	packages/database/package.json
#	packages/types/package.json
#	yarn.lock
@jaskfla jaskfla merged commit 1287db2 into dev Apr 4, 2024
43 checks passed
@jaskfla jaskfla deleted the rn-1112-bump-knex branch April 4, 2024 20:34
@jaskfla jaskfla mentioned this pull request Apr 4, 2024
1 task
@avaek avaek mentioned this pull request Apr 8, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants