twenty
twenty copied to clipboard
Merge secret keys into one
We currently have many access tokens:
ACCESS_TOKEN_SECRET=replace_me_with_a_random_string_access
LOGIN_TOKEN_SECRET=replace_me_with_a_random_string_login
REFRESH_TOKEN_SECRET=replace_me_with_a_random_string_refresh
FILE_TOKEN_SECRET=replace_me_with_a_random_string_refresh
I don't think it really improves security to have multiple tokens because if a bad actor gets access to the environment, he will probably get access to all variables. It could also make it more difficult to rotate.
We should probably merge this into a single APP_KEY
variable
Also we need to be extra careful about any code related to that key (e.g. if we encode a token without salt, it would be easy to brut force and guess the app key). So let's modify our danger.js script and add something like this (not tested).
async function checkAppKeyChangesInAllFiles() {
const modifiedFiles = danger.git.modified_files;
for (const filePath of modifiedFiles) {
const diff = await danger.git.diffForFile(filePath);
let appKeyChanged = false
if (!diff) continue; // Skip if there's no diff
appKeyChanged = appKeyChanged || diff.added.includes('APP_KEY') || diff.removed.includes('APP_KEY');
}
if (appKeyChanged) {
warn(`:warning: This PR modifies a line containing \`APP_KEY\` in \`${filePath}\`. It will need 2 reviewers.`);
}
}
// Run the check across all modified files
checkAppKeyChangesInAllFiles();
Yes please ! @FelixMalfait I'm not sure to understand how danger would detect it, we could instead prevent to use APP_KEY directly and to always use it through a method adding a salt. This would be easy to forbid through linter and would enforce usage of salt by design