-
Notifications
You must be signed in to change notification settings - Fork 83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Security implications of using authToken
inside app.json
#321
Comments
authToken
inside app.jsonauthToken
inside app.json
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 |
@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 ... |
@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: 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 {
"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 Please forgive my lack of familiarity with Expo. 🙂 It looks like the Either way, it shows a more secure pattern. The 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.
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, It's a bit confusing, but we use the actual
The result of
I can't agree more, but PR #324 might not provide the best transition moving off from the 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 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 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.
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...
What should I do if I'm affected? Remove the affected Sentry auth token (here), and configure a new token through If you want to create a new EAS Update, use 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. |
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
The text was updated successfully, but these errors were encountered: