athena-express icon indicating copy to clipboard operation
athena-express copied to clipboard

Adds bound parameters for enhanced security

Open fgheorghe opened this issue 4 years ago • 13 comments

  • Introduces bound parameter support by adding an optional second parameter to method query(), named 'queryParams' of type array, or .queryParams optional 'query' object key of the same type.
  • Updates documentation to reflect the availability of bound parameters

Examples: await athenaExpress.query('SELECT * FROM movies WHERE movie_title = ?', ['Spider-Man']); await athenaExpress .query({ sql: 'SELECT * FROM movies WHERE movie_title = ?', queryParams: ['Spider-Man']});

fgheorghe avatar Jan 10 '20 12:01 fgheorghe

Pull Request Test Coverage Report for Build 100

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 91: 0.0%
Covered Lines: 3
Relevant Lines: 3

💛 - Coveralls

coveralls avatar Jan 10 '20 12:01 coveralls

Hi, I was wondering about the status of this PR? Support for bound parameters is an important security feature... thanks!

mdesousa avatar Dec 04 '20 00:12 mdesousa

Hi @mdesousa - waiting for it to be merged as indeed it is useful!

fgheorghe avatar Dec 04 '20 10:12 fgheorghe

Thanks for your wonderful work in creating this library, @ghdna. Is there anything stopping this PR from being merged? This is a critical security feature.

jhkueh avatar Nov 18 '21 00:11 jhkueh

ok, let me look into this one next.

ghdna avatar Nov 19 '21 00:11 ghdna

sorry - it's been a while, i can look into updating the code and resolve merge conflicts at some point soon to make it easier for @ghdna

fgheorghe avatar Dec 13 '21 14:12 fgheorghe

PR updated to bring it in line with master @ghdna Please review

fgheorghe avatar Jan 12 '22 09:01 fgheorghe

Wondering the status on this? Came across this PR 3 years later as I was looking for this feature. Still relevant, please merge if possible!

lsharples1 avatar May 25 '23 19:05 lsharples1

What is the PR status? Athena supports parameterized queries already so it would be really nice to have that integrated in athena-express.

pierre-marieb avatar Aug 23 '23 10:08 pierre-marieb

I suspect this project is abandoned, and considering it doesn't properly escape parameters I'd say it's not secure enough to use. One option is to use code from my PR: https://github.com/fgheorghe/athena-express/tree/master

fgheorghe avatar Aug 23 '23 10:08 fgheorghe

That's too bad... we went the direction of integrating sqlstring the same way that you did but it would have been nice to have that PR officially merged.

But if it's no longer maintained we might have to go back to using the regular AWS SDK I guess

pierre-marieb avatar Aug 23 '23 11:08 pierre-marieb

Its maintained. The PR has conflicts. Once they are resolved, I can merge it

ghdna avatar Aug 23 '23 12:08 ghdna

Conflicts have been resolved a year ago, and since then new conflicts have been introduced. I can not re-issue a fix as I don't have time for it.

fgheorghe avatar Aug 23 '23 13:08 fgheorghe