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

new security rule: detect-child-process #252

Closed
HamletDRC opened this issue Sep 16, 2016 · 4 comments · Fixed by #855
Closed

new security rule: detect-child-process #252

HamletDRC opened this issue Sep 16, 2016 · 4 comments · Fixed by #855
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Domain: Security Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Rule Suggestion Adding a new rule that doesn't yet exist here or in TSLint core.
Milestone

Comments

@HamletDRC
Copy link
Member

https://github.com/nodesecurity/eslint-plugin-security/blob/master/rules/detect-child-process.js

@JoshuaKGoldberg JoshuaKGoldberg added Status: Accepting PRs Good First Issue 🙌 Howdy, neighbor! Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Domain: Security Type: Rule Suggestion Adding a new rule that doesn't yet exist here or in TSLint core. labels Jul 7, 2018
@jasperdanan
Copy link

I'd like to help, how can I contribute @JoshuaKGoldberg ?

I assume it's an implementation of an ESLint functionality to TSLint?

@JoshuaKGoldberg
Copy link

Heya @jasperdanan, glad to have you contribute!

Correct. The eslint-plugin-security rule looks good and we'd like to have an equivalent in tslint-microsoft-contrib so that TSLint users can get the same rule benefits.

This isn't quite a copy & paste job, since the ASTs for ESLint and TSLint are different, but the functionality should be about the same.

Is that enough info to go off of?

@jasperdanan
Copy link

jasperdanan commented Oct 16, 2018

Yep!
I'll try figuring out the code first.

Though I'm wondering if this https://github.com/Microsoft/tslint-microsoft-contrib/blob/master/src/nonLiteralRequireRule.ts is an implimentatiion of https://github.com/nodesecurity/eslint-plugin-security/blob/master/rules/detect-non-literal-require.js ?

Just so I can draw a parallel between the two Linter rules.

Where should I put the new rule? Under/src/ ?

@JoshuaKGoldberg
Copy link

if this https://github.com/Microsoft/tslint-microsoft-contrib/blob/master/src/nonLiteralRequireRule.ts is an implimentatiion of https://github.com/nodesecurity/eslint-plugin-security/blob/master/rules/detect-non-literal-require.js

Yup, that's correct. It looks like ours is checking for arrays passed to require, but otherwise they're roughly the same.

Where should I put the new rule? Under/src/ ?

Yup! #543 is a good example of what you'll want to add:

  • New rule under src/
  • Corresponding new test file under src/tests
  • New entry in README.md describing the rule
  • New line in tslint.json enabling the rule locally

The recommended_ruleset.js and tslint-warnings.csv should be auto-generated by some tasks within grunt all once you have a passing build.

Let me know if that's not the right or not enough information to go off of!

soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 4, 2019
soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 16, 2019
soon added a commit to soon/tslint-microsoft-contrib that referenced this issue Apr 24, 2019
JoshuaKGoldberg pushed a commit that referenced this issue Apr 24, 2019
* Added new `detect-child-process` rule (#252)

* Minor docs changes (#252)
@IllusionMH IllusionMH added this to the 6.2.0-beta milestone May 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Domain: Security Good First Issue 🙌 Howdy, neighbor! Status: Accepting PRs Type: Rule Suggestion Adding a new rule that doesn't yet exist here or in TSLint core.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants