getsentry / sentry-wizard

Sentry Project Setup Wizard
MIT License
190 stars 48 forks source link

sentry.properties not added on .gitignore for React Native #586

Open lucas-zimerman opened 3 months ago

lucas-zimerman commented 3 months ago

Environment

What version are you running 3.23.1

npx @sentry/wizard@latest -s -i reactNative

Steps to Reproduce

I have opened this issue to discuss if it's ideal or not to add sentry.properties to git ignore since it contains an authentication token.

After running the wizard and building the app, I noticed that sentry.property was created under both iOS/Android and that no git ignore rule was applied for it, so users could commit it without noticing it.

This file contains the user auth token:

auth.token=sntryu_375nnn10182qsr46s6r78s725q224rr2ao679120r3no33p9n496s6828o86q2nr // Invalid token.

defaults.org=sentry-sdks
defaults.project=sentry-react-native

defaults.url=https://sentry.io/

Expected Result

This file should be included on gitignore and sentry.properties not tracked by git.

What you thought would happen.

Actual Result

sentry.properties is tracked by git.

Lms24 commented 3 months ago

Good catch! I agree that whatever file we create that contains a secret should be .gitignore'd. This is in line with our SDK wizard specification.

Are you interested in opening a PR for this? cc @krystofwoldrich thoughts?

lucas-zimerman commented 3 months ago

Sure thing, I could open a PR for it.

Also for more context, when pushing a project with sentry.properties on github an application emailed me with the following message image

Lms24 commented 3 months ago

Let's do it, thanks!

zubko commented 2 months ago

I've just tried the new Sentry integration with "@sentry/react-native": "5.26.0" and the integration wizard. if I .gitignore sentry.properties then the build will fail on CI with:

Sentry Source Maps upload will include the release name and dist.
Sentry-CLI arguments: [/bitrise/src/packages/mobile/node_modules/@sentry/cli/bin/sentry-cli, react-native, gradle, --bundle, /bitrise/src/packages/mobile/android/app/build/generated/assets/createBundleP4hStagingReleaseJsAndAssets/index.android.bundle, --sourcemap, /bitrise/src/packages/mobile/android/app/build/generated/sourcemaps/react/p4hStagingRelease/index.android.bundle.map, --release, uk.co.pets4homes.staging@3.67.0+574175233, --dist, 574175233]
  WARN    2024-08-01 15:27:53.022331510 +00:00 Failed to find file referenced by SENTRY_PROPERTIES (/bitrise/src/packages/mobile/android/sentry.properties)
error: A project slug is required (provide with --project)

so far I've removed the auth token from the file and checked it in Git. I think this is not a good strategy to keep both the auth token (security sensitive but optional info) and some Sentry project identifiers (not so sensitive, but required) in the same file, then the file cannot be ignored that easily and then some other developer can accidentally Git push the auth token if they login on their machine and don't check their Git diff.

Lms24 commented 2 months ago

@zubko I'm lacking a bit of context in ReactNative land but I agree that having all paramers in one file isn't ideal. IIRC we discussed this before @krystofwoldrich do you remember specifics? Generally, the Sentry CLI also works with the SENTRY_AUTH_TOKEN environment variable so depending on your setup you can declare this env variable, for example in a .env file. Again, I'm not an RN expert, so Krystof knows more here.

krystofwoldrich commented 2 months ago

Thank you @zubko for the message, as @Lms24 mentioned we are aware of this, and will improve the experience in the future.

The best is to set SENTRY_AUTH_TOKEN env, but we can't do that automatically in the wizard due to security. Sadly the .env files behave differently across the RN build environments Xcode, Gradle, Expo...