Closed jamsch closed 1 year ago
The auth token is public either way, it's impossible to send something to Sentry without using an auth token in the frontend. Everything that's used by your app should be considered public.
Hi @kbrandwijk, I believe that the only thing the clients should be able to see is the DSN URL which just includes the public key. If users are able to access our auth tokens they'd be able to tamper with our account, such as:
# list organizations
curl https://sentry.io/api/0/organizations/ -H 'Authorization: Bearer <auth_token>'
# retrieve projects from the organization
curl https://sentry.io/api/0/organizations/{organization_slug}/projects/ -H 'Authorization: Bearer <auth_token>'
# retrieve issues from a project
curl https://sentry.io/api/0/projects/{organization_slug}/{project_slug}/issues/ -H 'Authorization: Bearer <auth_token>'
# upload a new sourcemap for a release
curl https://sentry.io/api/0/projects/{organization_slug}/{project_slug}/releases/{version}/files/ \
-H 'Authorization: Bearer <auth_token>' \
-H 'Content-Type: multipart/form-data' \
-F name=/demo/hello.min.js.map \
-F file=@hello.min.js.map
Please re-open this...could allow for leaking of private data included in logs...easily found and used via GitHub...auth token has more access than the dsn url...it shouldn't be possible to set authToken via json file, only env variable.
@jamsch i dont think it shows up. i just checked by logging Constants.expoConfig and the hooks
section does not appear.
@ci-vamp You may have to start with a fresh babel build cache. Seems to appear for me.
If you can just search for them in GitHub then it's probably not secure...
Just confirmed...there are tonnes of them out there... @kbrandwijk can you escalate this ...
Hang on. There's a difference between loading them as secrets in the hooks section of the config and loading them plain text / hard coding them in the config.
Absolutely never store secrets in source control, that is a best practice regardless of this plugin.
We load them as secrets (using .env / dotenv and EAS secrets) and do not see them when logging Constants in app. It could be because of the cache, I haven't tested it. I would find it weird that the Constants would capture the hooks section.
@GeorgeBellTMH it has been a while but don't the access tokens have a section for configuring scope? A token for this plugin should be scoped to just what is needed which afair is not equivalent to full control over the account / team / project.
Fully agreed...I just think it shouldn't even be an option to put them in the hooks section...it can just load the env variable in the background - which reduces the risk you are discussing of being able to print the in memory object and eliminates the risk of checking it in...even if the authToken is limited in scope you can still do a bunch of stuff more than just submitting a crash...https://docs.expo.dev/guides/using-sentry/ says you need: org:read, project:releases, and project:write ...
FYI this is with the "bare" workflow on iOS sim, build was completely wiped (no cache)
@GeorgeBellTMH good points man.
looks like those scopes are more broad than i had realized. project:write enables these (among others - havent checked them all):
@jamsch looks like these scope wont allow enumerating issues but it can do so for events (https://docs.sentry.io/api/events/list-a-projects-events/)
@ci-vamp Just bootstrapped a new Expo managed project
npx create-expo-app expo-with-sentry
cd expo-with-sentry
npx expo install sentry-expo
npx expo install expo-constants
@jamsch so it looks like this only affects managed projects?
Reopening this issue. My understanding is that Sentry has two configuration values: the DSN, which identifies an app and is not secret by virtue of being included in client applications, and the auth token, which is used to perform actions on behalf of a developer like uploading source maps and is secret.
Assuming the above is accurate: It is correct for client applications to access the DSN and so making the DSN available through Constants.expoConfig for instance is appropriate. Client applications should never have access to the auth token and it should always be kept secret on a developer’s computer or server. Whether or not developers consider their version control system (GitHub) to be private under their threat model, the auth token never should be made available to a client application.
Is this an accurate synopsis so far?
Yah, it may be better to put it in eas.json, or to just keep it always as a env var. There may be situations where even specific devs may not need it...but you are correct that it is dependent on risk profile. Would be good if documentation makes this clear, and existing users are reminded if they put it in a place where it could be shared widely.
Thanks for the confirmation. It sounds like there are several things to investigate or improve.
I think on the sentry side they probably should work on getting added to the GitHub secret scanner.... https://docs.github.com/en/code-security/secret-scanning/secret-scanning-patterns#supported-secrets-for-partner-alerts
for reference in case it helps, here is how we are loading secrets in app.config.js
import * as fs from 'fs';
import * as dotenv from 'dotenv';
// loads <root>/.env file into process.env, see <root>/.template.env for instructions
dotenv.config();
// This is the best way I've found to determine whether an EAS build command is being run, either locally or from the Expo CI server.
// Check for process.env.EAS_BUILD first as the Expo CI server doesn't include the other variable
const isEASBuild = process.env.EAS_BUILD || process.env.npm_lifecycle_script?.includes('eas build');
// NOTE: If being run through EAS build script, then the environment variables from eas.json will be automatically injected into the process.env.
// Process.env values will take priority. Otherwise we get the eas.json ourselves and manually parse the env variables from the correct environment
/**
* Secret values that must be loaded from:
* - local: .env (see .template.env for instructions)
* - EAS: secrets management @see https://docs.expo.dev/build-reference/variables/#using-secrets-in-environment-variables
*/
const secretEnvVars = {
SENTRY_AUTH_TOKEN: process.env.SENTRY_AUTH_TOKEN,
// other secrets
};
/**
* Public env vars that are exposed to the app through expo-constants
*/
let publicEnvVars = {
APP_ENV: process.env.APP_ENV,
SENTRY_DSN: process.env.SENTRY_DSN,
SENTRY_ORG: process.env.SENTRY_ORG,
SENTRY_PROJECT: process.env.SENTRY_PROJECT,
SENTRY_SET_COMMITS: process.env.SENTRY_SET_COMMITS,
// other public (exposed to app through Constants) vars
};
if (!isEASBuild) {
const rawData = fs.readFileSync('./eas.json');
// NOTE: env vars are loaded through the easConfig then must be placed in expo.extra object below
const easConfig = JSON.parse(rawData);
const easProfile = publicEnvVars.APP_ENV || 'development';
const easEnvVars = easConfig.build[easProfile].env || {};
publicEnvVars = { ...publicEnvVars, ...easEnvVars };
}
const config = {
expo: {
extra: {
eas: {
projectId: '<your project ID>',
},
...publicEnvVars,
},
hooks: {
postPublish: [
{
file: 'sentry-expo/upload-sourcemaps',
config: {
// https://docs.expo.dev/guides/using-sentry/#step-4-app-configuration
deployEnv: publicEnvVars.APP_ENV,
organization: publicEnvVars.SENTRY_ORG,
project: publicEnvVars.SENTRY_PROJECT,
setCommits: Boolean(publicEnvVars.SENTRY_SET_COMMITS),
authToken: secretEnvVars.SENTRY_AUTH_TOKEN,
},
},
],
},
}
}
and eas.json (w/ secrets stored in EAS secrets)
each environment extends production, sets any customizations and sets the APP_ENV in "environment"."env"."APP_ENV"
{
"build": {
"production": {
"channel": "production",
"ios": {
"resourceClass": "m1-medium"
},
"env": {
"APP_ENV": "production",
"SENTRY_DSN": "",
"SENTRY_ORG": "",
"SENTRY_PROJECT": "",
"SENTRY_SET_COMMITS": "true"
}
},
"otherEnv": {
"channel": "otherEnv",
"extends": "production",
"env": {
"APP_ENV": "otherEnvName"
}
},
}
Sentry security team here! 👋
For your shipped client applications, it is safe to include the DSN as this is used solely for uploading errors, transactions, etc. It does not allow anyone to make changes to your organization or projects in Sentry, nor does it facilitate reading data from your Sentry account.
You should keep the authToken
secret. This should not be shipped in your client applications. Depending on the scopes granted to the token, it can mutate organization settings, projects, read data, etc.
Please forgive my lack of familiarity with Expo. 🙂 It looks like the authToken
is used for creating a new release in Sentry and uploading sourcemaps? We have a published GitHub Action that can be used for this here, but it looks like Expo supports this natively in the postPublish
hook.
Either way, it shows a more secure pattern. The authToken
should be stored as a GitHub secret and loaded via an environment variable in your CI workflows to publish the release and push the sourcemaps -- not hardcoded and shipped in your client application.
As an aside, Sentry is working to improve our API tokens so we can better integrate with various secret scanning services. You can find our public RFC here if your interested. Our current API token format is prone to false positives. Hopefully in the near future we'll be able to auto-detect these accidental leaks and react quickly.
@mdtro Thank you for chiming in! That matches our understanding. We'll make some changes to the Expo docs to encourage using a secret store like GitHub secrets and minimize the likelihood of someone leaking their authToken on accident.
I took a look at your RFC too if you're looking for feedback. It is similar to the approach Expo takes/is thinking to take for access tokens. The idea to include info about the token type (user vs app) in the token sigil is nice. Some of the things we're still figuring out are whether to automatically revoke tokens, which might depend on context. For instance, the economic damage that can be done to a free plan account is less than the damage to a customer's account. And the damage that can be done with a bot access token is less than the damage that can be done with a personal access token. Another idea is to record the time a token was last used - even if the granularity is rounded to the day for performance reasons. I find it helpful that services like AWS show if a token hasn't been used recently.
Hi all! We take security seriously, that's why I wanted to follow up on some important security concerns raised here. We will go ahead and work on the 3 points ide mentioned here.
Both @jamsch and @ci-vamp mentioned that the
hooks
section within the app config is available within the app. (link)
The Expo app config is the main way to configure your app, and it's used by different parts of Expo when creating your app. Right now, we use 2 different types, prebuild
and public
(see here). While it's true that the hooks
part of the Expo config is available through expo-constants
in your app during development mode, it won't be in the production build.
It's a bit confusing, but we use the actual app.json
file as Constants.expoConfig
during development mode. But when creating the build, we use the public
version of this configuration within the app itself (see here). We'll take a closer look to see if we can bring development mode closer to the actual production version. If you want to validate this yourself, you can do that too ofc.
$ npx expo config --type public
in your project root.zip
file.
Payload/EXConstants.bundle/app.config
and see the JSON contentbase/assets/app.config
and see the JSON content.The result of npx expo config
, and the actual JSON files embedded in the build, do not contain the hooks
section of your app.json. It also doesn't matter what workflow you use, bare and managed are handled the same in EAS build.
@GeorgeBellTMH mentioned that Sentry auth tokens should not be searchable on public repositories.
I can't agree more, but PR #324 might not provide the best transition moving off from the authToken
property. I'll open a PR that actively discourages using the authToken
, and links to soon-to-be-updated docs on why it's discouraged.
We can publish that as a patch right away, since it won't break people's setup, while still raising awareness and pushing users to the better path. In the next major version, we can consider removing (at least) the authToken
property.
Thanks for raising the concerns, and we will get started on removing these concerns asap. Also, thanks for chiming in @mdtro, and thanks @ci-vamp for providing an example for app.config.js
. Feel free to raise any other concerns you might have.
Hope this helps!
@byCedric I just checked now -- this doesn't seem to apply to EAS update manifests. The Expo config for hooks are still accessible from this URL
https://u.expo.dev/[project-id]?runtime-version=[version]&channel-name=production&platform=android
It's also able to be console logged using:
import { manifest } from "expo-updates";
console.log(manifest);
As of now, we have the following PRs ready, and I'll make sure they get merged and deployed asap.
sentry-expo@6.1.1
eas-cli@3.8.1
Once merged, we'll probably announce this through our usual channels to ensure everyone is informed.
Does this affect my app?
The security issue only applies to you when you have...
authToken
directly inside the Expo config (app.json
or app.config.js
).authToken
inside the Expo config, using eas-cli@<=3.8.0
.What should I do if I'm affected?
Remove the affected Sentry auth token (here), and configure a new token through SENTRY_AUTH_TOKEN
(see EAS Build docs on how to configure secrets).
If you want to create a new EAS Update, use eas-cli@3.8.1
or higher, and create a new update.
Hope this helps!
Closing this issue as all fixes have been released and/or deployed. See the message above for more info.
I've spent a lot of time until I realized that secrets only work if you compile on expo servers. If you compile locally, there are no secrets, so the values are empty. Thanks.
Summary
Can someone clarify whether placing Sentry's authToken inside
app.config.ts
/app.json
would be accessible to users in production builds by a simple console.log ofConstants.expoConfig
?In the Expo documentation it mentions:
But this doesn't seem to apply to sentry-expo?
Reading the sentry-expo documentation the default recommendation is to place
authToken
inside app.json. But using expo-constants or expo-updates to log out the config seems to bundle the auth token in to production builds. The only way to avoid this is to use environment variables as well as settingauthToken: false
to avoid the console warnings, which seems like it's not recommended.Managed or bare workflow? If you have
ios/
orandroid/
directories in your project, the answer is bare!bare
What platform(s) does this occur on?
Android, iOS
SDK Version (managed workflow only)
47
Environment
expo-env-info 1.0.5 environment info: System: OS: macOS 13.1 Shell: 5.8.1 - /bin/zsh Binaries: Node: 16.15.1 - /usr/local/bin/node Yarn: 1.22.19 - /usr/local/bin/yarn npm: 8.12.1 - /usr/local/bin/npm Watchman: 2022.09.05.00 - /opt/homebrew/bin/watchman Managers: CocoaPods: 1.11.3 - /Users/xxxxx/.rbenv/shims/pod SDKs: iOS SDK: Platforms: DriverKit 22.2, iOS 16.2, macOS 13.1, tvOS 16.1, watchOS 9.1 IDEs: Android Studio: 2021.3 AI-213.7172.25.2113.9123335 Xcode: 14.2/14C18 - /usr/bin/xcodebuild npmPackages: expo: ~47.0.12 => 47.0.13 react: 18.1.0 => 18.1.0 react-dom: 18.1.0 => 18.1.0 react-native: 0.70.5 => 0.70.5 react-native-web: ~0.18.12 => 0.18.12 npmGlobalPackages: eas-cli: 3.6.1 expo-cli: 6.3.1 Expo Workflow: bare
Reproducible demo or steps to reproduce from a blank project