Add textlint and its config #428

sotayamashita · 2020-06-19T06:10:32Z

This PR covers issue #413

ThunderSon · 2020-06-20T14:47:18Z

@sotayamashita why is this in draft? And is this the same action as the action created by @kingthorin for the WSTG? (no issues with that, just asking).

kingthorin · 2020-06-20T15:21:03Z

Seems to be the same config file, textlint seems included in package.json. There does not appear to be any invocation/use though.

sotayamashita · 2020-06-21T06:54:16Z

@ThunderSon

why is this in draft?

I am sorry that I cannot have time to complete it.

And is this the same action as the action created by @kingthorin for the WSTG?

Yes, it is. I just copied from @kingthorin's WSTG.

sotayamashita · 2020-06-21T06:57:11Z

.gitignore

I thought it was our policy not to commit these lock files, so I added them.

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md

rbsec

Hi @sotayamashita.

Thanks for this PR - especially on the older cheat sheets there's a lot of...interesting capitalisation, so adding linting to prevent that in future is certainly a good thing. The amount of changes in this PR shows how inconsistent some of it was before.

While I'm happy with the capitalisation changes (apart from a few exceptions flagged in the review), the language changes are more of a problem. These have clearly been done with a find/replace, which has caused various problems such as incorrect capitalisation, broken sentence structure (for example, "allow list" is not a verb, but it's used by one in some places), renaming sections (which would break any links to them), and incorrectly naming external links to other pages or projects. We had a similar issue in #402, which ended up with some very strange phrasing such as describing a systems "accessible to administer".

Some of these changes are harmless as they only appear in text and actually read better (such as "he/she" to "they"), but others are more questionable, such as "regex" to "regular expression" (the term "regex" is actually more widely used, and there are some references to specific projects using that word which are now incorrect), and most significantly the use allow/block list.

If these kind of language changes are going to be implemented, that needs to be discussed and done properly (rewriting sentence structure, updating the style guidelines, ensuring that the language is consistent with other projects and references, fixing links broken by section title changes, etc), rather than just as a unilateral change made in a PR about linting.

My thoughts would be to remove those specific terms from the linter (and revert the associated changes), and then once this is merged the list of terms that we're checking for can gradually evolve over time (and as we spot common errors in the cheat sheets).

@ThunderSon @mackowski what are your thoughts?

rbsec · 2020-06-21T11:05:08Z

cheatsheets/Bean_Validation_Cheat_Sheet.md

@@ -76,7 +76,7 @@ When it comes to error handling, the Hibernate Validator returns a `BindingResul

 **Use**:

-Checks if the annotated string matches the regular expression regex considering the given flag match. Please visit [OWASP Validation Regex Repository](https://owasp.org/www-community/OWASP_Validation_Regex_Repository) for other useful regex's.
+Checks if the annotated string matches the regular expression regular expression considering the given flag match. Please visit [OWASP Validation regular expression Repository](https://owasp.org/www-community/OWASP_Validation_Regex_Repository) for other useful regex's.


The project is called "OWASP Validation Regex Repository" - so this change is incorrect. I've seen a couple like this - perhaps the linting should ignore [link text]?

@rbsec I have set it to ignore all link elements.

rbsec · 2020-06-21T11:05:08Z

.textlintrc

+			// List of terms
+			"terms": [
+				// Brands
+				"Airbnb",


Why does this include random unrelated companies that have no relevant to the project? Seems like advertising..

This is an upstream carry-over which is itself an upstream (or well source) carryover.

This part of the list contains brand names to ensure they're properly capitalized. It could be trimmed, though obviously the thing is they won't be caught if they aren't in the config. So while they might not be applicable at the moment that doesn't mean they won't be in the future.

rbsec · 2020-06-21T11:05:08Z

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md

 ```

-Please note there are some JavaScript functions that can never safely use untrusted data as input - **EVEN IF JAVASCRIPT ESCAPED!**
+Please note there are some JavaScript functions that can never safely use untrusted data as input - **EVEN IF JavaScript ESCAPED!**


This sentence was in capitals for emphasis, so putting JavaScript in proper case in the middle of it looks wrong.

I agree this causes it to look out of place. However, I'd also suggest there must be a better way to emphasize things than to YELL at the reader 😉

I'll use https://github.com/textlint/textlint-filter-rule-comments to prevent it from being converted

rbsec · 2020-06-21T11:05:08Z

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md

@@ -410,9 +410,9 @@ The `X-XSS-Protection` header has been deprecated by modern browsers and its use
 | Data Type | Context                                  | Code Sample                                                                                                        | Defense                                                                                                                                                                                        |
 |-----------|------------------------------------------|--------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | String    | HTML Body                                |  `<span>UNTRUSTED DATA </span>`                                                                          | HTML Entity Encoding (rule \#1).                                                                                                                                                               |
-| String    | Safe HTML Attributes                     | `<input type="text" name="fname" value="UNTRUSTED DATA ">`                                               | Aggressive HTML Entity Encoding (rule \#2), Only place untrusted data into a whitelist of safe attributes (listed below), Strictly validate unsafe attributes such as background, id and name. |
+| String    | Safe HTML Attributes                     | `<input type="text" name="fname" value="UNTRUSTED DATA ">`                                               | Aggressive HTML Entity Encoding (rule \#2), Only place untrusted data into a allow list of safe attributes (listed below), Strictly validate unsafe attributes such as background, ID and name. |


The HTML property is "id", not "ID".

"an allow..."

id could be inline code fenced with backticks.

rbsec · 2020-06-21T11:05:08Z

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md

 | String    | GET Parameter                            | `<a href="/site/search?value=UNTRUSTED DATA ">clickme</a>`                                               | URL Encoding (rule \#5).                                                                                                                                                                       |
-| String    | Untrusted URL in a SRC or HREF attribute | `<a href="UNTRUSTED URL ">clickme</a> <iframe src="UNTRUSTED URL " />`                                   | Canonicalize input, URL Validation, Safe URL verification, Whitelist http and https URL's only (Avoid the JavaScript Protocol to Open a new Window), Attribute encoder.                        |
+| String    | Untrusted URL in a SRC or HREF attribute | `<a href="UNTRUSTED URL ">clickme</a> <iframe src="UNTRUSTED URL " />`                                   | Canonicalize input, URL Validation, Safe URL verification, allow list http and HTTPS URLs only (Avoid the JavaScript Protocol to Open a new Window), Attribute encoder.                        |


This sentence doesn't make sense "allow list" is not a verb

The whole sentence is horrible to start with.

I'd suggest a re-write such as:

Canonicalize input, validate URLs, verify safe URLs, allow only HTTP(S) URLs (avoid the JavaScript protocol to open a new window), use an attribute encoder.

(That might not be perfect, I haven't read through the entire content for context. But, hopefully you get the idea.)

rbsec · 2020-06-21T11:05:08Z

.textlintrc

+				//	"server side"
+				//],
+				[
+					"smartphone(s)?",


These words are not synonyms, "smartphone" is a subset of "mobile phone". Should be removed.

rbsec · 2020-06-21T11:05:08Z

.textlintrc

+					"build system(s)?",
+					"build tool$1"
+				],
+				//[


rbsec · 2020-06-21T11:05:08Z

.textlintrc

+				"jQuery",
+				"LinkedIn",
+				"Lodash",
+				"MacBook",


Again, why random brands in here?

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md

rbsec · 2020-06-21T11:05:08Z

.textlintrc

+				"WebAssembly",
+				"WebStorm",
+				"WordPress",
+				"YouTube",


I'm not too familiar with textlint, but my understanding of the format here is that:

Individual words/phrases must have that exact capitalisation or they're be flagged

Arrays will flag the first term (which is a regex) as incorrect, and recommend the second term as a replacement

Is this correct?

If so, it would be good to have a short comment at the top of this file explaining that, so it's clear how to add new terms

I am gonna investigate how to do that.

mackowski · 2020-06-29T15:33:05Z

@sotayamashita will you be able to fix issues that @rbsec highlighted and read all the changes again to review their correctness?

sotayamashita · 2020-07-01T01:45:27Z

@mackowski I am sorry not to replay quickly. I am gonna fix it until tomorrow.

@rbsec Thank you for the detailed review!

mackowski · 2020-07-01T07:06:49Z

No problem at all, thank you!

sotayamashita · 2020-07-03T04:32:38Z

I am sorry I am working now.

sotayamashita · 2020-07-11T06:51:46Z

cheatsheets/Authorization_Testing_Automation_Cheat_Sheet.md

@@ -400,7 +400,7 @@ Access issues detected using the BASIC USER point of view:

 Even if the authorization matrix is stored in a human readable format (XML), it can be interesting to provide an on-the-fly rendering representation of the XML file in order to facilitate the review, audit and discussion about the authorization matrix in order to spot potential inconsistencies.

-The Following XSL stylesheet can be used:
+The Following XSL style sheet can be used:


Should be stylesheet

rbsec

Hi @sotayamashita

Thanks for the changes - I'd made a few small comments where the grammar has ended up a bit strange or where a few changes were missed when reverting.

There's still a few comments left in the textlintrc file (would be nice to have a few comments in there explaining how to add stuff, and cleaning out some junk terms we don't need), but other than those things this looks nearly ready to merge.

Thanks

cheatsheets/File_Upload_Cheat_Sheet.md

cheatsheets/Nodejs_Security_Cheat_Sheet.md

cheatsheets/Pinning_Cheat_Sheet.md

cheatsheets/Transaction_Authorization_Cheat_Sheet.md

cheatsheets/User_Privacy_Protection_Cheat_Sheet.md

cheatsheets/Virtual_Patching_Cheat_Sheet.md

cheatsheets/Vulnerable_Dependency_Management_Cheat_Sheet.md

.textlintrc

sotayamashita · 2020-07-12T02:26:53Z

@rbsec Thank you for taking the time to read through it, even though I made some major changes. 🙇 I hope it is ready to go.

rbsec · 2020-07-12T10:41:44Z

@sotayamashita thanks for making those changes - looks good to me.

ThunderSon

@kingthorin and @rejahrehim when possible, your review on the yml and package.json files would be great to ensure that it doesn't bother the link checker.

kingthorin

Looks good to me.

kingthorin · 2020-07-14T00:28:04Z

If you want to tackle the "client-" / "server-" side thing the Web Security Testing Guide has settled on hyphenated variants after twitter and slack polling. Details here: OWASP/wstg#526

rbsec · 2020-07-14T12:26:53Z

@kingthorin I'd agree on the hyphenated versions (which seem to be much more popular).

Let's get this merged first, then we can start adding in extra terms like that.

Add textlint and its config

Loading status checks…

2aa334c

Format text

Loading status checks…

eb5e75c

sotayamashita reviewed Jun 21, 2020

View changes

.gitignore

yarn.lock

Comment on lines +12 to +13

This comment has been minimized.

Sign in to view

sotayamashita Jun 21, 2020
Author Contributor

I thought it was our policy not to commit these lock files, so I added them.

sotayamashita reviewed Jun 21, 2020

View changes

cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.md Show resolved Hide resolved

sotayamashita marked this pull request as ready for review Jun 21, 2020

sotayamashita requested review from mackowski, rbsec and ThunderSon as code owners Jun 21, 2020

rbsec suggested changes Jun 21, 2020

View changes

sotayamashita reviewed Jul 11, 2020

View changes

sotayamashita added 6 commits Jul 11, 2020

update

Loading status checks…

962f448

update

Loading status checks…

4a9f1b0

update

Loading status checks…

d877b83

installing node modules with npm ci instead of installing global modules

3b6c079

Update

Loading status checks…

47fa7ff

Update

Loading status checks…

1a8aeae

sotayamashita requested a review from rbsec Jul 11, 2020

rbsec suggested changes Jul 11, 2020

View changes

update

Loading status checks…

25b24c2

sotayamashita requested a review from rbsec Jul 12, 2020

rbsec approved these changes Jul 12, 2020

View changes

ThunderSon reviewed Jul 12, 2020

View changes

kingthorin approved these changes Jul 14, 2020

View changes

ThunderSon approved these changes Jul 14, 2020

View changes

ThunderSon merged commit c22bf1d into OWASP:master Jul 14, 2020
3 checks passed

3 checks passed

link-check
Details

lint
Details

Publishing Check
Details

sotayamashita deleted the sotayamashita:topic/check-terms branch Jul 14, 2020

sotayamashita mentioned this pull request Nov 25, 2020

Fix a notation for iOS bitrise-io/devcenter#598

Merged

OWASP / CheatSheetSeries

Add textlint and its config #428

Add textlint and its config #428

sotayamashita commented Jun 19, 2020 •

edited by ThunderSon

ThunderSon commented Jun 20, 2020

kingthorin commented Jun 20, 2020

sotayamashita commented Jun 21, 2020 •

edited

This comment has been minimized.

rbsec left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mackowski commented Jun 29, 2020

sotayamashita commented Jul 1, 2020 •

edited

mackowski commented Jul 1, 2020

sotayamashita commented Jul 3, 2020

This comment has been minimized.

rbsec left a comment

sotayamashita commented Jul 12, 2020

rbsec commented Jul 12, 2020

ThunderSon left a comment

kingthorin left a comment

kingthorin commented Jul 14, 2020

rbsec commented Jul 14, 2020

OWASP / CheatSheetSeries

Add textlint and its config #428

Add textlint and its config #428

Conversation

sotayamashita commented Jun 19, 2020 • edited by ThunderSon

ThunderSon commented Jun 20, 2020

kingthorin commented Jun 20, 2020

sotayamashita commented Jun 21, 2020 • edited

This comment has been minimized.

sotayamashita Jun 21, 2020 Author Contributor

rbsec left a comment

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

sotayamashita Jul 11, 2020 Author Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

kingthorin Jun 21, 2020 Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

kingthorin Jun 21, 2020 Contributor

This comment has been minimized.

sotayamashita Jul 2, 2020 • edited Author Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

kingthorin Jun 21, 2020 Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

kingthorin Jun 21, 2020 Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

sotayamashita Jul 11, 2020 Author Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

sotayamashita Jul 2, 2020 • edited Author Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

rbsec Jun 21, 2020 Contributor

This comment has been minimized.

sotayamashita Jul 2, 2020 Author Contributor

mackowski commented Jun 29, 2020

sotayamashita commented Jul 1, 2020 • edited

mackowski commented Jul 1, 2020

sotayamashita commented Jul 3, 2020

This comment has been minimized.

sotayamashita Jul 11, 2020 Author Contributor

rbsec left a comment

sotayamashita commented Jul 12, 2020

rbsec commented Jul 12, 2020

ThunderSon left a comment

kingthorin left a comment

kingthorin commented Jul 14, 2020

rbsec commented Jul 14, 2020

sotayamashita commented Jun 19, 2020 •

edited by ThunderSon

sotayamashita commented Jun 21, 2020 •

edited

sotayamashita Jun 21, 2020
Author Contributor

rbsec Jun 21, 2020
Contributor

sotayamashita Jul 11, 2020
Author Contributor

rbsec Jun 21, 2020
Contributor

kingthorin Jun 21, 2020
Contributor

rbsec Jun 21, 2020
Contributor

kingthorin Jun 21, 2020
Contributor

sotayamashita Jul 2, 2020 •

edited

Author Contributor

rbsec Jun 21, 2020
Contributor

kingthorin Jun 21, 2020
Contributor

rbsec Jun 21, 2020
Contributor

kingthorin Jun 21, 2020
Contributor

rbsec Jun 21, 2020
Contributor

sotayamashita Jul 11, 2020
Author Contributor

rbsec Jun 21, 2020
Contributor

sotayamashita Jul 2, 2020 •

edited

Author Contributor

rbsec Jun 21, 2020
Contributor

rbsec Jun 21, 2020
Contributor

sotayamashita Jul 2, 2020
Author Contributor

sotayamashita commented Jul 1, 2020 •

edited

sotayamashita Jul 11, 2020
Author Contributor