Skip to content
This repository has been archived by the owner on Jul 12, 2019. It is now read-only.

Censor sensitive numbers #152

Merged
merged 1 commit into from
May 3, 2019
Merged

Censor sensitive numbers #152

merged 1 commit into from
May 3, 2019

Conversation

codebycaleb
Copy link
Contributor

Fixes two problems:

  1. When creating the sensitive bank, we add entries for secrets,
    their URI-encoded value, and their base64-encoded value. For
    numeric secrets (e.g. a bank account id of 8675309), the line
    where we base64-encoded the secret failed because Buffer.from
    won't accept a numeric value.
  2. When replacing values from the sensitive bank, only Strings
    were considered. This still worked for replacing values inside
    of a string (e.g. Your account number is :censored:) but not
    as a standalone value in an object (e.g. {account: 314159}).

Testing

  1. zapier init censortest --template=minimal
  2. Add the following lines inside of the App definition in index.js:
  authentication: {
    type: 'session',
    fields: [
      {
        key: 'topping',
        type: 'string',
        label: 'Topping',
        helpText: 'What is your favorite pizza topping?',
        required: true,
      },
      {
        key: 'account',
        type: 'integer',
        required: false,
        computed: true,
      }
    ],
    test: (z, bundle) => z.request({
      url: 'http://httpbin.org/post',
      method: 'POST',
      headers: {'X-ID': 314159265},
      body: {id: 314159265},
    }).then(resp => resp.json.data),
    connectionLabel: '{{account}}',
    sessionConfig: {
      perform: (z, bundle) => ({account: 314159265})
    },
  },
  1. zapier push
  2. Create an auth
  3. In GL, for the request logs for the POST to http://httpbin.org/post, you'll see multiple null references logged.
  4. In your app's package.json, change the zapier-platform-core dependency to zapier/zapier-platform-core#fix-censor-numbers; npm i
  5. Change the line in package.json back to 8.0.1
  6. Run SKIP_NPM_INSTALL=1 zapier push
  7. Press the "Test" button for your auth
  8. In GL, the input data should now have the value :censored:9:9cb84e8ccc: properly censored everywhere.

Fixes two problems:

  1. When creating the sensitive bank, we add entries for secrets,
     their URI-encoded value, and their base64-encoded value. For
     numeric secrets (e.g. a bank account id of 8675309), the line
     where we base64-encoded the secret failed because Buffer.from
     won't accept a numeric value.
  2. When replacing values from the sensitive bank, only Strings
     were considered. This still worked for replacing values inside
     of a string (e.g. `Your account number is :censored:`) but not
     as a standalone value in an object (e.g. `{account: 314159}`).
@codebycaleb codebycaleb requested a review from xavdid May 1, 2019 12:57
@codebycaleb
Copy link
Contributor Author

For reference, https://zapier-platform.slack.com/archives/C2S5DGF6G/p1556655567057200 was what caused this investigation. I wasn't ever able to reproduce the exact same error on Zapier (though to be honest, I never made a trigger or create to test with), but I did pseudo-recreate it via npm testing. If you run the committed test on the current version of core (8.0.1), you'll see the TypeError: "value" argument must not be a number error, which was actually the root cause of the original report.

Copy link
Contributor

@xavdid xavdid left a comment

Choose a reason for hiding this comment

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

sweet, great catch. thanks a bunch!

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

Successfully merging this pull request may close these issues.

None yet

2 participants