feat(v2): add single quote to CSV output

[wanlingt]

Problem

Works towards #133

Solution

Use regex expressions to determine if the field value starts with a formula character. If so, prefix it with a single quote. The only exceptions to this are field values that only contain numbers, + or - e.g. +65, -1.

Design doc here

Breaking Changes

• Yes - this PR contains breaking changes
  • Downloaded CSVs might have cells that start with an additional single quote, if any of their field values start with a formula character

Tests

• Enter a value starting with a formula character into a storage form's short text, long text and table fields. Download the responses. These values should be prefixed with a single quote.
• Repeat the same for a form's feedback.

[karrui]

lgtm, but wait for one more set of eyes pls :P.

@amitogp @mantariksh would we need to communicate this change to users too? not sure what we decided on.

[timotheeg]

Minor comments:

• No need to use the RegExp constructor, unless we are constructing a pattern from parts
• Pure numbers regexp should have an end anchor as well. Otherwise in its current form, it still allows formulas like this: +3*SUM(A1) (in fact do add a test for an input like that please)
• Checking for .* at the end of a regexp can be discarded

Suggested change

	export const PURE_NUMBER_REGEXP = new RegExp('^[+-][0-9]+')
	export const FORMULA_INJECTION_REGEXP = new RegExp('^[+=@-].*')
	export const PURE_NUMBER_REGEXP = /^[+-]\d+(\.\d+)?$/
	export const FORMULA_INJECTION_REGEXP = /^[+=@-]/

Maybe we can simplify PURE_NUMBER_REGEXP to /^[+-][\d.]+$/ ? That's not exact number regexp, but it's safe from an injection perspective.

[justynoh]

thinking about whether we should check just /^[+-.\d]*$/ for expressions like "-4+2-0" or something like that, since i guess those are formulas but i don’t think they actually pose any risk (correct me if i’m wrong). It might be simpler to describe to admins too, to explain the behavior of what we add quotes to and what we don’t

[wanlingt]

@justynoh yeah I don't think they pose any risks if they are just digits and + or -. Didn't come across that being mentioned in any of the sites which talked about formula injection - the malicious commands all contained at least 1 alphabet. thanks for spotting that case!
Made the change in 858f4dc, decided to go with /^[+-.\d]+$/

[wanlingt]

@timotheeg thanks tim! added the additional test here c0b1511

[wanlingt]

@timotheeg 's offline comment: - is a range separator, so it should only be added at the end of the regexp. otherwise it captures ASCII characters between + and .
e0da940

[justynoh]

out of curiosity, then, if they all contain at least 1 alphabet, why not just use a single regex /^[-+@=].*[a-zA-Z]/ haha. Maybe there is a case I'm not seeing

[wanlingt]

@justynoh that regex wouldn't capture other symbols (e.g. |) and formulas like =SUM(A1). also I think it's better to play it safe and not have Excel react to any formulas at all (other than ones with numbers and + | -, which we know are harmless).

[amitogp]

@karrui @wanlingt the plan is to release it with react and announce it as a security improvement along with other react enhancements. Something like: "Enhanced security to stop malicious inputs to form responses." @pearlyong please help in adding it in our react enhancements. @kennethchangOPENGOV will need your help in adding details of this to our guide.

[wanlingt]

@karrui @wanlingt the plan is to release it with react and announce it as a security improvement along with other react enhancements. Something like: "Enhanced security to stop malicious inputs to form responses." @pearlyong please help in adding it in our react enhancements. @kennethchangOPENGOV will need your help in adding details of this to our guide.

@amitogp I've created new issues for these followup actions
@pearlyong - #4970
@kennethchangOPENGOV - #4971

[karrui]

@wanlingt there's a bug here right? there should be a single quote in the expected line

[karrui]

or actually should not have anything here since there was no change to AngularJS's feedback csv generator.

Suggested change

	const insertedLine = `${insertedCreatedDate},'${feedback.comment},${feedback.rating}`
	const insertedLine = `${insertedCreatedDate},${feedback.comment},${feedback.rating}`

[wanlingt]

my bad, you are right! this tests for feedback csv generator in AngularJS not React. Will have to add a separate test for React

[wanlingt]

fixed in your PR 8b33acc