Skip to content
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

EAS CD fingerprinting fails when using file-based secrets for Android configuration #315

Open
tharakadesilva opened this issue Dec 5, 2024 · 10 comments
Assignees

Comments

@tharakadesilva
Copy link
Contributor

tharakadesilva commented Dec 5, 2024

Topic and scope of discussion

EAS CD fingerprint validation fails when using file-based secrets due to path differences between GitHub CI environment and EAS build environment

Motivation

The continuous deployment process is breaking because the fingerprint generated in the GitHub CI environment doesn't match the fingerprint calculated during the EAS build when using file-based secrets. This prevents successful deployment of builds even when there are no actual changes to the application code.

Additional context

I'm encountering an issue with the continuous deployment fingerprinting system when trying to securely handle the google-services.json file for Android builds.

Current Setup:

  1. Using expo-github-action/continuous-deploy-fingerprint for CD
  2. Following security best practices by not committing google-services.json to git
  3. Implemented the recommended approach of using environment variables (https://docs.expo.dev/eas/environment-variables/#file-environment-variables):
    googleServicesFile: process.env.GOOGLE_SERVICES_JSON || "./google-services.json"
  4. Created a file-based secret using EAS:
    eas secret:create --scope project --name GOOGLE_SERVICES_JSON --type file --value ./google-services.json

Problem:
The fingerprint validation fails because:

  1. In the GitHub CI environment, the fingerprint is calculated using one file path configuration
  2. When the build runs in EAS, the secret is materialized to a different random file path
  3. This path difference in app.config.js causes the fingerprints to mismatch
  4. As a result, EAS build fails with a fingerprint validation error

Questions:

  1. What's the recommended way to handle file-based secrets with the fingerprinting system to ensure consistent paths between CI and EAS environments?
  2. Should the fingerprinting system ignore or normalize paths for file-based secrets?
  3. Are there alternative approaches to managing the google-services.json file that would work better with the fingerprinting system?
@mrkpatchaa
Copy link

I just wanted to try the new fingerprint action and came accros the same issue @tharakadesilva . Did you find a workaround?

@tharakadesilva
Copy link
Contributor Author

Hi @mrkpatchaa, I ended up doing the super secure thing of committing the file.

@mrkpatchaa
Copy link

@tharakadesilva 🤐😅

@mrkpatchaa
Copy link

@wschurman maybe you can take a look? It doesn't seem to have been any activity on the repo since 3 months now.
Thank you.

@mrkpatchaa
Copy link

A workaround I just found is to ignore fingerprint check on the whole expo config.
https://github.com/expo/expo/blob/main/packages/%40expo/fingerprint/src/sourcer/SourceSkips.ts#L40
As mentioned in the readme https://www.npmjs.com/package/@expo/fingerprint
But it's not the solution because it can lead to issues

  /**
   * Skip the whole ExpoConfig.
   * Prefer the other ExpoConfig source skips when possible and use this flag with caution.
   * This will potentially ignore some native changes that should be part of most fingerprints.
   * E.g., adding a new config plugin, changing the app icon, or changing the app name.
   */

@tharakadesilva
Copy link
Contributor Author

tharakadesilva commented Dec 27, 2024

I took a look at the code and have two proposals:

Option 1

We add an option here and remove it here similar to how we handle other config options.

Downside: This will affect the file-based fingerprinting we do here. Since we want the fingerprint to change when this file changes, I don't recommend this approach.

Option 2

This approach has two parts:

  1. Move this results.push to after the externalFiles have been resolved here
  2. Before the results.push, delete all external file keys available in expoConfig

While this solves one problem, we'd still have a fingerprint mismatch since we're fingerprinting the file here.

Solution

We can resolve this by using standardized filenames instead of full paths for known external files holding secrets

  • ios.google-services-file becomes ios-google-services-file
  • android.google-services-file becomes android-google-services-file

We can also get this mapping as a dict input from the user config to accommodate future secret file references in app.json.

This would, however, make the file contents to appear in the fingerprint diff (though the current code would too..).

Considerations

  • We'll need exact google-services.json files in CI environments to avoid content mismatches. One potential solution is to ignore diff failures between EAS and CI env for known external files containing credentials (or if a mapping is provided by the user).

Recommendation

I recommend proceeding with Option 2.

If this approach sounds good to everyone, I'll prepare a PR. As always, feedback and alternative approaches are welcome!

@mrkpatchaa
Copy link

@tharakadesilva I like the option 2 with a slightly difference.
Instead of moving the push, just create another object that will be added to the content. That way, We avoid any edge case in the future.

...
expoConfig = normalizeExpoConfig(config.exp, options);
const expoConfigResult = normalizeExpoConfig(config.exp, options); // <--
delete expoConfigResult.android.googleServicesFile; // <-- 
delete expoConfigResult.ios.googleServicesFile; // <-- 
loadedModules = stdoutJson.loadedModules;
results.push({
    type: 'contents',
    id: 'expoConfig',
    contents: (0, Utils_1.stringifyJsonSorted)(expoConfigResult), // <--
    reasons: ['expoConfig'],
});

I am currently testing this code, and it's working for now.

I wonder if there is any other key apart from googleServicesFile to exclude. If yes We can think of moving it to a configuration in sourceskips.

@Kudo
Copy link
Contributor

Kudo commented Jan 3, 2025

hi there! much appreciated for digging the problem and fingerprint code.

for googleServicesFile, i agree both of you. since we cover googleServicesFile in external files already, i think we can safely remove the key in normalizeExpoConfig. feel free to send us a pr to do that.

for the other fields that may appear in app config with absolute paths (absolute paths from eas file based secret like process.env.SECRET_FILE), i think we should prevent using absolute paths in app config. for example, in @mrkpatchaa's repro example. we can do relative paths like

const path = require('path');

module.exports = {
  expo: {
    ios: {
      googleServicesFile: path.relative(__dirname, process.env.GOOGLE_SERVICES_PLIST),
    },
    android: {
      googleServicesFile: path.relative(__dirname, process.env.GOOGLE_SERVICES_JSON),
    }
  },
};

not sure if we can have a heuristic approach to turn all absolute paths to relative paths in fingerprint.

lastly, fwiw, other than the risky SourceSkips.ExpoConfigAll, i had expo/expo#33610 that you can do some customization to transform app config before generating fingerprint hash. trying to add fingerprint.config.js with the following contents

/** @type {import('@expo/fingerprint').Config} */
const config = {
  fileHookTransform: (source, chunk, isEndOfFile, encoding) => {
    // Remove googleServicesFile
    if (source.type === 'contents' && source.id === 'expoConfig') {
      const config = JSON.parse(chunk);
      delete config.android.googleServicesFile;
      delete config.ios.googleServicesFile;
      return JSON.stringify(config);
    }

    return chunk;
  },
}

module.exports = config;

@Kudo Kudo self-assigned this Jan 3, 2025
@mrkpatchaa
Copy link

Hi @Kudo I've tested the fileHookTransform config and it works like a charm. 💪🏾
I am wondering if it's still needed to make the pull request for googleServicesFile, or We just need to menton that somewhere in the documentation.

@Kudo
Copy link
Contributor

Kudo commented Jan 3, 2025

I am wondering if it's still needed to make the pull request for googleServicesFile, or We just need to menton that somewhere in the documentation.

having expo/expo#33926 as a heuristic solution. maybe it would be better than specific to googleServicesFile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants