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

We should highlight that using prisma.raw() with parameters is not secure and recommend using prisma.raw`` #449

Closed
Jolg42 opened this issue Mar 25, 2020 · 13 comments · Fixed by #728
Labels
docs Documentation creation, updates or corrections

Comments

@Jolg42
Copy link
Member

Jolg42 commented Mar 25, 2020

Problem

Users are using prisma.raw() like

const data = await prisma.raw(
  `SELECT * FROM "ProviderItemAttribute" WHERE "provider_item" = ${root.id} AND "user" = ${auth.user.id} limit 1;`,
);

This example is using prisma.raw() the pure text version so there is no security around parameters.

Only raw`` is secure because it's using https://github.com/blakeembrey/sql-template-tag

Solution

In this case it would be recommended to do

const data = await prisma.raw`
	SELECT * FROM "ProviderItemAttribute" WHERE "provider_item" = ${root.id} AND "user" = ${auth.user.id} limit 1;
`;

This should be highlighted in the docs (and examples?)
We also can think about how to warn users that are using prisma.raw() or even disable it under a flag?

Note prisma.raw`` parameters do not work as of today with PostgreSQL see prisma/prisma-client-js#595

@ryancwalsh
Copy link

@Jolg42 Thanks for posting this issue.

I'm trying to do a count in group by and have been told that Prisma doesn't yet support it and that I need to use raw.

I'm wondering what you'd recommend for fixing this, as my ${whereAnd} causes the error below:

export const tagsByLabel = async (params) => {
  let whereAnd = ' '
  if (params) {
    const { searchTerm } = params
    if (searchTerm) {
      whereAnd = ` AND t.title LIKE '%${searchTerm}%' `
    }
  }
  const tagsByLabelResult = await db.raw`SELECT t.*, COUNT(pt.B) as count
  FROM tag t
  LEFT JOIN _PrincipleToTag pt
  ON t.id = pt.B
  WHERE t.userId = ${userIdFromSession}
  ${whereAnd}
  GROUP BY t.id
  ORDER BY count DESC, t.title ASC;`
  return tagsByLabelResult
}
Error fetching tags Error: GraphQL error: Raw query failed. Code: `1`. Message: `near "?": syntax error`     at new ApolloError (bundle.esm.js:63)

Thank you so much for any advice.

@Sytten
Copy link

Sytten commented May 4, 2020

Currently its very hard because the sql tag is not exported and there is no way to send params with the function (using the ()). See this PR prisma/prisma#2311

@ryancwalsh
Copy link

@Sytten Thank you. I subscribed to that issue.

@arvindell
Copy link
Contributor

arvindell commented May 26, 2020

Adding variables with prisma.raw`` is not working either on MySQL.

@pantharshit00
Copy link
Contributor

@AlexVilchis Can you please open a new issue with a reproduction for that? That should not happen

@janpio janpio transferred this issue from prisma/prisma Jun 12, 2020
@steebchen
Copy link
Contributor

I still have concerns that it's way too easy to accidentally use parentheses when you actually don't want to. Would it be possible to maybe introduce a second method which just allows template literal inputs (e.g. raw`SELECT 1` ) which disallows using parentheses (e.g. raw(`SELECT 1`), and a new method which just allows passing in data unescaped via a string rawWithoutEscape (similar to react's dangerouslySetInnerHtml.

@steebchen steebchen added the web/candidate Candidate for next Web Sprint label Jun 15, 2020
@steebchen
Copy link
Contributor

IMO, this is not just a docs issue – highlighting it in the docs is NOT enough. In fact, the current docs are long outdated, which shows how many users actually read them before using a feature. It's not enough.

@steebchen
Copy link
Contributor

I just realised that when you let autocompletion do its job, it will default to the parentheses method, which means no escaping will be done. That's very dangerous

@janpio
Copy link
Member

janpio commented Jun 15, 2020

Then please open an issue in the appropriate place instead of talking to yourself in the docs repo @luca :D

@steebchen
Copy link
Contributor

steebchen commented Jun 15, 2020

Could we just transfer it back to prisma/prisma? You moved it here in the first place.

edit: although @Jolg42's original issue was not really clear that it's not just a docs issue:

We also can think about how to warn users that are using prisma.raw() or even disable it under a flag?

I created a new issue to clarify the concerns: prisma/prisma-client-js#727

@steebchen steebchen removed the web/candidate Candidate for next Web Sprint label Jun 15, 2020
@janpio
Copy link
Member

janpio commented Jun 15, 2020

I moved it here as it was tagged as devrel + docs (I assume, that is why things are moved here), and thus belongs here.

@mhwelander mhwelander added the docs Documentation creation, updates or corrections label Jun 16, 2020
@mhwelander mhwelander added the web/candidate Candidate for next Web Sprint label Jun 24, 2020
@mhwelander
Copy link
Contributor

@nikolasburk nikolasburk removed the web/candidate Candidate for next Web Sprint label Jul 7, 2020
@mhwelander mhwelander linked a pull request Aug 3, 2020 that will close this issue
@Jolg42
Copy link
Member Author

Jolg42 commented Aug 4, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation creation, updates or corrections
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants