Rails6 #13
Rails6 #13
Conversation
|
I will try to review it over the weekend. |
config/secrets.yml
Outdated
| secret_key_base: a44341d23f54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 |
config/secrets.yml
Outdated
| @@ -16,7 +16,10 @@ development: | |||
| test: | |||
| secret_key_base: a44a31d23f54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 | |||
|
|
|||
| staging: | |||
| secret_key_base: a44a31d4gf54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 | |||
config/settings.yml
Outdated
| staging: | ||
| secret_key_base: a44a31d4gf54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 | ||
| production: | ||
| secret_key_base: a44341d23f54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 |
There was a problem hiding this comment.
Secrets cannot be hardcoded in code. Period.
Doesn't matter if they are in secrets.yml or settings.yml
In fact, it is worse when they are in settings.yml
| gem 'sqlite3', '~> 1.3.6' | ||
|
|
||
| gem 'sdoc'#, '~> 0.4.0', group: :doc | ||
| gem 'sqlite3'#, '~> 1.3.6' |
There was a problem hiding this comment.
Please pin the versions of the dependencies to the ones which are currently installed. Unpinned dependencies will break application as soon as there is a breaking change
And we don't have any CI build to check if something is breaking or not
| // const images = require.context('../images', true) | ||
| // const imagePath = (name) => images(name, true) | ||
|
|
||
| console.log('Hello World from Webpacker') |
config/initializers/dragonfly.rb
Outdated
| Dragonfly.app.configure do | ||
| plugin :imagemagick | ||
|
|
||
| secret "7f65c8d4317cc27ae282bb064e9093beb2c255c5efac060685fa3957b0b020f4" |
config/secrets.yml
Outdated
|
|
||
| test: | ||
| secret_key_base: a44a31d23f54d081b9f3737ab886048a01e2188e659d53ea608b997bb34a6dcc77b91b729a607807fb6771b6cfcd76be09bfdd0f07a6021c2c0969a07fa11755 | ||
| secret_key_base: <%= Settings.secret.test.secret_key_base %> |
There was a problem hiding this comment.
Development and test secret keys should be kept in place to ease automated tests
There was a problem hiding this comment.
... except cloud service API keys.
|
Weren't we using yarn? There is package-lock.json which is npm specific. Not used in yarn |
|
Since github is parsed by bots for those leaks, keys are now considered as compromised and need to be replaced ASAP |
|
Adding reverts for more dirty commits history ? |
|
@fcartegnie We need the reverts to make the PR mergable. The damage is done. The only course of action is to make the PR mergable by removing the keys from code and recreate new keys and disband old ones |
No, do+undos don't have to land in any commit history. |
|
When PR is squashed and merged, it won't be in commit history |
|
github squash does all commits as one, that's not a solution |
|
There are 7 commits in the PR, 2 of them reverting the secrets, which make them 5 commits effectively. None of the 5 commits do radically different things to keep them separate from each other. All commits are part of same PR of making the project compatible with rails 6 If the commits needed to be separate, they would have been better as separate PRs, but that's not the case here. We merge several PRs with multiple commits related to the same issue across projects in FOSSASIA. We just want that PR to atomically solve one thing so we can individually revert it. Requiring separate commits, in this case, is not helpful |
No description provided.