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

Prevent secrets from sending in email #502

Merged
merged 2 commits into from
Oct 22, 2018

Conversation

spelcaster
Copy link
Contributor

@spelcaster spelcaster commented Oct 17, 2017

  • Added the methods:
    • setSecretRules: user can configure a set of named rules to test the
      e-mail content;
    • filterSecrets: this method test all the e-mail content with the
      rules configured in setSecretRules, nothing is done if secretRules
      is empty, and if a rule is found in the e-mail content, then a
      exception is thrown;
  • The method filterSecrets is called before the request to the API and
    should prevent sensitive data leakage;

@mbernier edit:
closes #496

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 17, 2017
@spelcaster
Copy link
Contributor Author

#496

@SendGridDX
Copy link
Collaborator

SendGridDX commented Oct 17, 2017

CLA assistant check
All committers have signed the CLA.

@spelcaster
Copy link
Contributor Author

spelcaster commented Oct 17, 2017

Sample file:

// mail_sample.js
const sgMail = require('./packages/mail');
sgMail.setApiKey(process.env.SENDGRID_API_KEY);

/**
any of the following models will work

sgMail.setSecretRules({
  name: 'apikey_regex',
  pattern: /SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+/
});

sgMail.setSecretRules({
  name: 'apikey_string',
  pattern: 'SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+'
});

sgMail.setSecretRules(/SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+/);

sgMail.setSecretRules('SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+');

sgMail.setSecretRules([
  'test',
  /aloha/,
  {
    name: 'apikey_string',
    pattern: 'SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+'
  },
  {
    name: 'apikey_regex',
    pattern: /SG\.[a-zA-Z0-9_]+\.[a-zA-Z0-9-]+/
  }
]);
*/

const msg = {
  to: 'name@example.com',
  from: 'name@example.com',
  subject: 'Sending with SendGrid is Fun',
  text: 'SG.Xx1XX.Xx1XX-Xx1X-X1xXX-XX11X-XxX and easy to do anywhere, even with Node.js',
  html: '<strong>and easy to do anywhere, even with Node.js</strong>',
};
sgMail.send(msg);

clee
clee previously requested changes Feb 14, 2018
Copy link

@clee clee left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @spelcaster! Unfortunately, it looks like some other changes that were merged into the sendgrid-nodejs codebase have resulted in a merge conflict, so we need you to rebase your code. Once your PR applies cleanly to master, please let us know so that we can merge it!

@clee clee added status: code review complete and removed status: code review request requesting a community code review or review from Twilio labels Feb 14, 2018
- Added the methods:
  - setSecretRules: user can configure a set of named rules to test the
    e-mail content;
  - filterSecrets: this method test all the e-mail content with the
    rules configured in setSecretRules, nothing is done if secretRules
    is empty, and if a rule is found in the e-mail content, then a
    exception is thrown;
- The method filterSecrets is called before the request to the API and
  should prevent sensitive data leakage;
- The method setSecretRules can now receive an object, a string and an
  array of string and/or objects, then it will try to standardize the
  rules to the following structure:
  [
    {
        name: 'rule_name', // optional
        pattern: /pattern/ // required
    }
  ]

- Refactored filterSecrets to use the new secretRules types, changed the
  exception from string to Error type and the error message is a template
  string now;
@spelcaster
Copy link
Contributor Author

@clee done!.

I've squashed one of the commits and changed how my variables were declared, I'm using const and let now

@clee
Copy link

clee commented Feb 17, 2018

@spelcaster Awesome! Thanks for taking care of this so quickly. I’ll check with the rest of the team to see how soon we can get this merged.

@thinkingserious thinkingserious added type: twilio enhancement feature request on Twilio's roadmap status: ready for deploy code ready to be released in next deploy and removed status: code review complete labels Feb 27, 2018
@spelcaster
Copy link
Contributor Author

@clee, Is there any change to be done here yet?

@thinkingserious thinkingserious dismissed clee’s stale review June 11, 2018 17:32

Request has been completed.

@thinkingserious
Copy link
Contributor

Hi @spelcaster,

Everything is good on your end. This is now on our backlog for merge.

With Best Regards,

Elmer

@thinkingserious thinkingserious merged commit 0220500 into sendgrid:master Oct 22, 2018
@thinkingserious
Copy link
Contributor

Hello @spelcaster,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: ready for deploy code ready to be released in next deploy type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent secrets from sending in email
4 participants