[Config #68] create new file

[github-actions[bot]]

Adding a new config file

• Initialized by creating or updating the coressponding issue
• Auto-generated by create-pull-request

[Abby-Wheelis]

@shankari @JGreenlee - Draft generated using the form! When I met with Leidy about the ppt today I was also able to walk her through filling out this form. Of note we have mostly default settings in here right now, so it will need to be updated.

We decided to revisit the decision of autogenerated vs not later.

Also, it seems like the form may be more confusing to someone seeing it for the first time than I would have thought, I might be curious to see how other users react or what we could do to make it easier for people to fill out.

[shankari]

@Abby-Wheelis I asked @sebastianbarry to help out with tweaking the form given: (i) fresh pair of eyes (ii) Laos not yet picked up. Maybe you can give him the feedback and he can try to tweak it. I might also have him work with some of the in-flight MOUs/potential partners to get more feedback.

[shankari]

@sebastianbarry can you also add the IT ticket for configuring the IT ticket for this config? I wonder if ServiceNow has a way to make API calls to auto-add the DNS?

[JGreenlee]

I am pretty sure survey_info > trip_labels should be ENKETO (not MULTILABEL)

The rest of survey_info will also change significantly – I will modify and add onto it later

[JGreenlee]

A first draft for what survey_info might look like:

...
"survey_info": {
  "surveys": {
    "NonEVTripSurvey": {
      "formPath": "<URL>/gpg-non-ev-trip-v1.xml",
      "version": 1,
      "compatibleWith": 1,
      "dataKey": "manual/trip_user_input",
      "labelTemplate": { "en": "Answered" }
    },
    "EVRoamingTripSurvey": {
      "formPath": "<URL>/gpg-ev-roaming-trip-v1.xml",
      "version": 1,
      "compatibleWith": 1,
      "dataKey": "manual/trip_user_input",
      "labelTemplate": { "en": "Answered" }
    },
    "EVReturnTripSurvey": {
      "formPath": "<URL>/gpg-ev-return-trip-v1.xml",
      "version": 1,
      "compatibleWith": 1,
      "dataKey": "manual/trip_user_input",
      "labelTemplate": { "en": "Answered" }
    },
    "UserProfileSurvey": {
      "formPath": "<URL>/gpg-onboarding-v1.xml",
      "version": 1,
      "compatibleWith": 1,
      "dataKey": "manual/demographic_survey",
      "labelTemplate": { "en": "Answered" }
    }
  },
  "buttons": {
    "trip-label": [
      {
        "surveyName": "NonEVTripSurvey",
        "not-filled-in-label": { "en": "Gas Car Survey" },
        "showsIf": {
          "conditions": [{
            "logic": "not",
            "function": "startsWith",
            "args": ["TRIP.bt_sensed_mode", "e_car"]
          }]
        }
      },
      {
        "surveyName": "EVRoamingTripSurvey",
        "not-filled-in-label": { "en": "EV Survey" },
        "showsIf": {
          "conditions": [
            {
              "function": "startsWith",
              "args": ["TRIP.bt_sensed_mode", "e_car"]
            },
            {
              "logic": "not",
              "function": "pointIsWithinBounds",
              "args": ["TRIP.end_loc.coordinates", [
                [39.717158, -105.117432],
                [39.718706, -105.114554]
              ]]
            }
          ],
          "logic": "and"
        }
      },
      {
        "showsIf": {
          "conditions": [
            {
              "function": "startsWith",
              "args": ["TRIP.bt_sensed_mode", "e_car"]
            },
            {
              "function": "pointIsWithinBounds",
              "args": ["TRIP.end_loc.coordinates", [
                [39.717158, -105.117432],
                [39.718706, -105.114554]
              ]]
            }
          ],
          "logic": "and"
        },
        "surveyName": "EVReturnTripSurvey",
        "not-filled-in-label": { "en": "EV Survey" }
      }
    ]
  },
  "trip-labels": "ENKETO"
},
...

This would outline a flexible way to conditionally show surveys depending on trip characteristics.

bt_sensed_mode doesn't exist yet and it depends on how we choose to represent bluetooth-sensed information in the data model. I thought maybe it'd be something like e_car_<Fleet vehicle #>. startsWith() and pointIsWithinBounds() would need to be defined in the code of e-mission-phone. We could include other functions there too, like equals, gt, lt. An easy use case I could see is a survey for long-distance trips if distance > threshold, else a short-distance survey

[JGreenlee]

One other approach I considered, which would be a lot less verbose, is actually putting entire JavaScript conditional expressions in the JSON as text. It would allow us to do a lot more without changes to e-mission-phone.

But it would require using eval() to parse them, which is not recommended unless you have complete control over what goes into eval() because it opens the door to injection attacks.

We generally do have control over the config files, since we control this repo. But, the files get downloaded during runtime so it seems like a bad idea.

[JGreenlee]

@shankari @jiji14 for visibility

[shankari]

@JGreenlee I am really torn here.

Option 1 is essentially creating a new expression language syntax using JSON, which seems wasteful, and not very general. Option 2 uses eval which is generally not recommended

However, I do think that the risks with eval don't really apply to us. eval is mainly a risk when invoked on strings provided by a user. In particular, before the JSON methods were added to javascript, people used to parse JSON by using eval(json_string). Which could result in https://xkcd.com/327/

In our case, we review the configs before we merge them. This is the same process that we use to merge code, which can do a lot more than eval. So I don't see the additional risk.

If we wanted to be extra paranoid and not have to explain this everytime somebody says "why do you use eval?", we can try to use a safer, but still powerful enough alternative such as mathjs. Note that mathjs no longer uses eval under the hood, although it is not 100% secure to arbitrary inputs. https://mathjs.org/index.html

A user could try to inject malicious JavaScript code via the expression parser. The expression parser of mathjs offers a sandboxed environment to execute expressions which should make this impossible. It’s possible though that there are unknown security vulnerabilities, so it’s important to be careful, especially when allowing server side execution of arbitrary expressions.

[JGreenlee]

Maybe I was being overly cautious - I don't know that much about what kind of attacks would be possible.

I thought there might be a way that the downloaded config could be 'spoofed' by the network. If the user was on public Wi-Fi that was controlled by a malicious party, is there a way the network could relay a tampered config?

---

There is one other problem I've realized we would face with the eval approach, which somewhat limits its usefulness.

I'd like to be able to declare some function in our codebase and reference it in the config, being able to use anything that's in scope at the point that the eval() is executed. Something like:

{
...
"showIf": "pointIsWithinBounds(timelineEntry.end_loc.coordinates, [39.717158, -105.117432], [39.718706, -105.114554])"
...

But when we bundle with Webpack, variable names and function names get minified / randomized. It will not know what pointIsWithinBounds or timelineEntry are. I was thinking we could put any potential functions in an object and look them up by key (the keys would stay the same). But the name of the object would be randomized too. At some point we'd have to have some specific strings to grab onto.

It would be something more like:

{
...
"showIf": "FN.pointIsWithinBounds(ENTRY.end_loc.coordinates, [39.717158, -105.117432], [39.718706, -105.114554])"
...
}

Before performing the eval() we'd have to replace FN and ENTRY with the actual variable names. (by doing some weird thing to look up the name of a variable by the variable itself, like https://stackoverflow.com/a/52598270)

I think that's kind of clunky. Although maybe not as clunky as writing if/else blocks in JSON

[JGreenlee]

Something else that's annoying is that JSON only has single-line strings and no way to add comments. Inline expressions are not going to be very easily readable.

[shankari]

@JGreenlee

I thought there might be a way that the downloaded config could be 'spoofed' by the network. If the user was on public Wi-Fi that was controlled by a malicious party, is there a way the network could relay a tampered config?

Not really. We always use a https URL to download the data, which provides protection against man-in-the-middle attacks. Basically, the site provides a certificate that (through a chain up to a root) proves that it is in fact the endpoint and then all communication is effectively encrypted with the certificate.

The public WiFi concerns are primarily when http connections were a lot more prevalent. https://security.stackexchange.com/questions/1525/is-visiting-https-websites-on-a-public-hotspot-secure#1527

(we should do some adversarial testing to ensure that if certificate of the host from which the config is downloaded has expired, we fail the download, but I assume that modern webviews will handle this properly)

I'd like to be able to declare some function in our codebase and reference it in the config, being able to use anything that's in scope at the point that the eval() is executed. Something like:

I believe we can do this with MathJS by having our codebase define the function(s) in mathjs (see the example

>> f(x, y) = x ^ y
f(x, y)
>> f(2, 3)
8

However, I am not sure how we can pass in variables to mathjs, all the examples are super simple.

[shankari]

To answer my own question, we can pass in objects and then refer to object properties https://mathjs.org/examples/objects.js.html

const scope = {
  obj: {
    prop: 42
  }
}

// retrieve properties using dot notation or bracket notation
print(evaluate('obj.prop', scope)) // 42
print(evaluate('obj["prop"]', scope)) // 42

[JGreenlee]

I think we want functions that can perform string manipulations as well as calculations, and I don't think MathJS would do that (e.g. it wouldn't know how to evaluate startsWith) But the scope idea would work well. How about this other package that does that? https://www.npmjs.com/package/scoped-eval

[shankari]

I think we want functions that can perform string manipulations as well as calculations, and I don't think MathJS would do that (e.g. it wouldn't know how to evaluate startsWith)

You can extend mathjs with arbitrary functions https://github.com/josdejong/mathjs/blob/cee9deb3772900b7ed03b9987f2a338cc595912d/examples/import.js#L3

But the scope idea would work well. How about this other package that does that? https://www.npmjs.com/package/scoped-eval

Mathjs with arbitrary functions is essentially the same as this, ~so I am fine with scoped-eval as well!~

EDIT: mathjs is currently maintained and has 13k stars. scoped-eval has not been updated for 2 years and has 5 stars. Given that, I would vote for using mathjs because of maintenance and community support

[shankari]

It also looks like the library is not maintained because it is not that hard to re-implement using new Function(...).call(scope) https://stackoverflow.com/questions/543533/restricting-eval-to-a-narrow-scope

[JGreenlee]

If we did MathJS, we'd have to make a wrapper for startsWith and any other vanilla JS function we want to use. Any time in the future if we decided we wanted to use a different vanilla JS function, we'd have to make changes to e-mission-phone to make wrappers and import them. If we're going to evaluate expressions at runtime, wouldn't it be easier for those expressions to just be vanilla JS?

It also looks like the library is not maintained because it is not that hard to re-implement using new Function(...).call(scope) https://stackoverflow.com/questions/543533/restricting-eval-to-a-narrow-scope

this method uses with and I think that won't work in ES6 because strict mode is on.

The other one I saw was https://stackoverflow.com/a/59600907 If we did that, I think we just have to use this to access values in the scope.

{
...
"showIf": "this.pointIsWithinBounds(this.timelineEntry.end_loc.coordinates, [39.717158, -105.117432], [39.718706, -105.114554])"
...
}

[shankari]

Interesting.

Yup, I agree that new Function('this.....').bind() is the way to go. Not sure if we want to use a library (e.g. vm-browserify) to implement it - might be overkill.

All this talk about scope is giving me flashbacks to angular. There was a $scope.$eval which evaluated templating language (subset of full javascript).

That make me think that maybe a templating library might also be an option if we want to use a library. The templating library will also avoid having us use the full power of eval and new Function since it builds a separate AST and works with a subset of the syntax. And we use templating extensively already.

e.g. something like https://handlebarsjs.com/ but better integrated with React?

[shankari]

Although, I looked through the handlebars code, and guess what I found https://github.com/handlebars-lang/handlebars.js/blob/25c696b8891e65f7f7e8405ecdf0a6b17808d237/spec/env/node.js#L23

function safeEval(templateSpec) {
  /* eslint-disable no-eval, no-console */
  try {
    return eval('(' + templateSpec + ')');
  } catch (err) {
    console.error(templateSpec);
    throw err;
  }
  /* eslint-enable no-eval, no-console */
}

I think that eval or new Function is fine in our particular context, where we approve the "code" that is included in the config files.

[JGreenlee]

I found something better - a concise 2-liner which uses the Function() constructor without with()

const scopedEval = (script, scope) =>
  Function(...Object.keys(scope), script)(...Object.values(scope));

scopedEval("console.log(foo)", {foo: 'bar'})
// logs 'bar'

[shankari]

@jgu2 for visibility to create the environment