Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[expo-cli] detect git changes on credentials.json update #2482

Merged
merged 2 commits into from
Aug 21, 2020

Conversation

wkozyra95
Copy link
Contributor

@wkozyra95 wkozyra95 commented Aug 21, 2020

Why

Writing credentials to the project directory is potentially dangerous for inexperienced users, we want to detect situations where files created by us are not gitignored and warn users to take action.

How

We are checking if the file is visible to git but untracked, doing that this way ensures that only new files will trigger that warning.
We don't warn in other cases because if the file is tracked or staged it means that the user added it intentionally, plus we can't easily distinguish cases where credentials are encrypted and we don't want to warn the user in that case.

Test plan

run sync command in the turtle-v2-example repo with and without credentials.json

@wkozyra95 wkozyra95 requested a review from dsokal August 21, 2020 10:23
async function isFileUntrackedAsync(path: string): Promise<boolean> {
const withUntrackedFiles = await gitStatusAsync({ showUntracked: true });
const trackedFiles = await gitStatusAsync({ showUntracked: false });
const pathWithouLeadingDot = path.replace(/^\.\//, ''); // remove leading './' from path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const pathWithouLeadingDot = path.replace(/^\.\//, ''); // remove leading './' from path
const pathWithoutLeadingDot = path.replace(/^\.\//, ''); // remove leading './' from path

Comment on lines 172 to 173
withUntrackedFiles.includes(pathWithouLeadingDot) &&
!trackedFiles.includes(pathWithouLeadingDot)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
withUntrackedFiles.includes(pathWithouLeadingDot) &&
!trackedFiles.includes(pathWithouLeadingDot)
withUntrackedFiles.includes(pathWithoutLeadingDot) &&
!trackedFiles.includes(pathWithoutLeadingDot)

function displayUntrackedFilesWarning(newFilePaths: string[]) {
if (newFilePaths.length === 1) {
log.warn(
`File ${newFilePaths[0]} is currently untracked, remember to add it to .gitignore or to encrypt it.`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`File ${newFilePaths[0]} is currently untracked, remember to add it to .gitignore or to encrypt it.`
`File ${newFilePaths[0]} is currently untracked, remember to add it to .gitignore or to encrypt it (e.g. with git-crypt).`

log.warn(
`Files ${newFilePaths.join(
', '
)} are currently untracked, remember to add them to .gitignore or to encrypt them.`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
)} are currently untracked, remember to add them to .gitignore or to encrypt them.`
)} are currently untracked, remember to add them to .gitignore or to encrypt them (e.g. with git-crypt).`

@dsokal
Copy link
Contributor

dsokal commented Aug 21, 2020

Remember to add an entry to the changelog!

@wkozyra95 wkozyra95 merged commit 7cd8e3f into master Aug 21, 2020
@wkozyra95 wkozyra95 deleted the @wkozyra95/detect-git-change-one-credentials-update branch August 21, 2020 13:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants