Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added tfsec as a security linter #199

Merged
merged 11 commits into from
Mar 10, 2023

Conversation

puzzler7
Copy link
Contributor

Added tfsec as a security linter. Includes a small fix to testing that prevents accidentally linting the json output of trunk check.

Copy link
Contributor

@sxlijin sxlijin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but why are we deleting so much of package-lock.json?

Also not sure why I follow .json -> .out.json to be important

@puzzler7
Copy link
Contributor Author

Re: package-lock.json: I'm not sure - when I initially made the PR, CI told me to run npm install, and I did and pushed the result. I don't know what the implications of that are.

Re: .json -> .out.json: tfsec runs on terraform files, which are .tf and .tf.json files. Additionally, it runs on the parent directory, rather than an individual file. When running the tests before this change, the test harness would run tfsec on the file aws.in.tf, and write the output to aws.in.tf.json in the same directory, at which point tfsec would attempt to run on the newly created file and error.

@puzzler7 puzzler7 requested a review from sxlijin March 10, 2023 00:48
Copy link
Contributor

@sxlijin sxlijin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's back out the package-lock.json changes - I suspect you ran into some kind of transient error, because we haven't modified package.json.

@@ -453,7 +453,7 @@ export class TrunkDriver {
*/
async runCheckUnit(targetRelativePath: string, linter: string): Promise<TestResult> {
const targetAbsPath = path.resolve(this.sandboxPath ?? "", targetRelativePath);
const resultJsonPath = `${targetAbsPath}.json`;
const resultJsonPath = `${targetAbsPath}.out.json`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment here explaining the tf.json thing

@puzzler7 puzzler7 enabled auto-merge (squash) March 10, 2023 21:44
@puzzler7 puzzler7 merged commit 8d26afc into main Mar 10, 2023
@puzzler7 puzzler7 deleted the maverick/trunk-5026-security-tool-tfsec branch March 10, 2023 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants