Fix data stores object parsing

[andrewjschuang]

Primary fix:

• [x] Using Function to evaluate JSON / JS objects - regex failed on some cases: on time values (containing :) and if there were spaces in between the key/value (e.g. {key : "value"} - note space before :)

Refactors:

• [x] return parsedValue whenever possible
• [x] new value propDefinition
• [x] change some logic of summary exports (affects order of execution only)
• [x] minor changes

Resolves #3515.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
pipedream-docs	✅ Ready (Inspect)	Visit Preview	Sep 2, 2022 at 11:09AM (UTC)
pipedream-docs-redirect-do-not-edit	✅ Ready (Inspect)	Visit Preview	Sep 2, 2022 at 11:09AM (UTC)

[dylburger]

Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:

• [ ] Create components to address specific use cases whenever possible
• [ ] Component keys should follow the format app_name_slug-slugified-component-name
• [ ] Components should follow the standard directory structure
• [ ] Prefer Node.js client libraries to REST APIs
• [ ] When making API requests, handle pagination to ensure all data / events are processed
• [ ] Use secret props to capture sensitive data
• [ ] Props and methods should be defined in app files whenever possible
• [ ] Document methods with JS Docs
• [ ] Use optional props whenever possible, and set a default value where you can
• [ ] Use async options to accept user input wherever possible

[andrewjschuang]

@dylburger @vellames-turing @feyzullah would love your inputs on possible security issues - commit 1452494c

[feyzullah]

@dylburger @vellames-turing @feyzullah would love your inputs on possible security issues - commit 1452494

Hi @andrewjschuang we had a convo about using eval last time. We decided not to use eval because if a user input be a malicious function, eval would run that function no matter what and we have no control over it. After that @vellames-turing added and used xss package for this reason. I think using xss and JSON.parse would be safe.

[andrewjschuang]

Hi @andrewjschuang we had a convo about using eval last time. We decided not to use eval because if a user input be a malicious function, eval would run that function no matter what and we have no control over it. After that @vellames-turing added and used xss package for this reason. I think using xss and JSON.parse would be safe.

Function would limit the access of scopes and variables at least, so it's safer than eval. I'm not sure how Data Stores are implemented - if it's just a user-limited container would it be an issue to expose this?

I've used this array for testing:

[
  "1",
  2,
  3.5,
  {
    key: "value",
  },
  new Date(),
  console.log("Hello!"),
  { "time" : "18:01:31" },
]

Result:

Note that console.log is executed and returns null, so that's what is being saved into the data store.

I've added a while(true) test, but I don't know why it throws an exception and saves everything as a string, which is nice.

Result:

Don't really know about other types of injected code.

[andrewjschuang]

Hi @andrewjschuang Everything looks great. However I consider you should change versions in the rest of components as well since you are touching data_stores.app.mjs. Don't you think?

Thanks, I don´t think it's necessary since the changed parts of the app file don't affect the other components.

[vunguyenhung]

Hello everyone, I have tested this PR and there're some test cases failed or needed improvement.

Please check the test report below for more information Data_Stores_3515_2202.pdf

[andrewjschuang]

Hello everyone, I have tested this PR and there're some test cases failed or needed improvement.

Please check the test report below for more information Data_Stores_3515_2202.pdf

Thanks, should be fixed!

[vunguyenhung]

Hi everyone, all test cases are passed! Ready for release!

Test report Data_Stores_3515_1829.pdf

[andrewjschuang]

/approve