datamapper / do

DataObjects
147 stars 74 forks source link

DataObjects logs database password on database error #18

Closed rsutphin closed 12 years ago

rsutphin commented 12 years ago

When there's a database exception thrown DO includes the database password in the logs. Example:

 DM: ERROR:  duplicate key value violates unique constraint "tsu_pkey"
DETAIL:  Key (tsu_id)=(.) already exists. (code: 83906754, sql state: 23505, query: INSERT INTO "tsu" ("sc_id", "psu_id", "tsu_id", "tsu_name") VALUES ('200000XX', '200000XX', '.', '.'), uri: postgres:mdes_warehouse:ACTUAL_PASSWORD@server:5432mdes_warehouse_working?username=mdes_warehouse&database=mdes_warehouse_working&adapter=postgres&host=server&port=5432&password=ACTUAL_PASSWORD)

I'm seeing this with DO 0.10.7 and do_postgres 0.10.7. I'm using DO within DataMapper 1.2.0. I mentioned this issue on the datamapper mailing list and it was suggested that it's a DO bug.

myabc commented 12 years ago

@dbussink @dkubb Are you in agreement we should elide this information from our logging?

dbussink commented 12 years ago

Yeah, we should not output this information in exceptions etc. so we should fix this behavior.

mkristian commented 12 years ago

definitely this info should stay away from log files !

dkubb commented 12 years ago

@myabc I agree, we definitely should not display this information in exception output.

The question is, where should the change be made and who should make it? @dbussink and @myabc, are you able to change the C and Java drivers respectively, or would someone else be better for making these changes?

systemed commented 6 years ago

This only half-fixes the issue. In @rsutphin's original report, uri contained:

postgres:mdes_warehouse:ACTUAL_PASSWORD@server:5432mdes_warehouse_working?username=mdes_warehouse&database=mdes_warehouse_working&adapter=postgres&host=server&port=5432&password=ACTUAL_PASSWORD

The first occurrence of ACTUAL_PASSWORD is now removed, but the second one is still present, serialised by iterating through the query attribute.

This can be fixed by changing the relevant line to

string << "?" << query.select {|k,v| k!='password' }.map do |key, value|

(I know this is seriously old code, but logging passwords is a seriously big bug. ;) )