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

Commenter returning "Ignoring - change not part of the current PR" for all comments #90

Open
trevorvoncannon opened this issue Dec 8, 2022 · 26 comments · May be fixed by #94
Open

Commenter returning "Ignoring - change not part of the current PR" for all comments #90

trevorvoncannon opened this issue Dec 8, 2022 · 26 comments · May be fixed by #94

Comments

@trevorvoncannon
Copy link

trevorvoncannon commented Dec 8, 2022

I have a workflow (below) that correctly parses my .tf files in a PR, but never actually comments on it. Already looked into permissions issues - no problem here. I have also cycle through various arguments to see if they would yield the result I wanted, but again I've had no luck.

name: tfsecPOC
on: 
  pull_request:
jobs:
  tfsec-compliance:
    name: tfsec compliance check
    runs-on: [self-hosted, vault]
    steps:
      - name: Checkout repo
        uses: actions/checkout@v3
      - name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@main
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          tfsec_args: --debug

I just get this output at the end of my run

1 file(s) written: results.json
+ commenter
Starting the github commenter
Working in repository terraform
Working in PR 1333
TFSec found 5 issues
Working in GITHUB_WORKSPACE /github/workspace/
Preparing comment for violation of rule aws-ec2-add-description-to-security-group in ./terratest-poc/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-ec2-add-description-to-security-group-rule in ./terratest-poc/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-ec2-enable-at-rest-encryption in ./terratest-poc/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-ec2-enforce-http-token-imds in ./terratest-poc/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-ec2-no-public-ingress-sgr in ./terratest-poc/main.tf
Ignoring - change not part of the current PR

I can see where this error is coming from, but I am not able to see what the exact issue with the results.json file is.

for _, result := range results {
		result.Range.Filename = workingDir + strings.ReplaceAll(result.Range.Filename, workspacePath, "")
		comment := generateErrorMessage(result)
		fmt.Printf("Preparing comment for violation of rule %v in %v\n", result.RuleID, result.Range.Filename)
                err := c.WriteMultiLineComment(result.Range.Filename, comment, result.Range.StartLine, result.Range.EndLine)
		if err != nil {
			// don't error if its simply that the comments aren't valid for the PR
			switch err.(type) {
			case commenter.CommentAlreadyWrittenError:
				fmt.Println("Ignoring - comment already written")
				validCommentWritten = true
			case commenter.CommentNotValidError:
				fmt.Println("Ignoring - change not part of the current PR")
				continue
			default:
				errMessages = append(errMessages, err.Error())
			}
		} else {
			validCommentWritten = true
			fmt.Printf("Commenting for %s to %s:%d:%d\n", result.Description, result.Range.Filename, result.Range.StartLine, result.Range.EndLine)

Action command output

/usr/bin/docker run --name cbb2812a0638619a79478ab2aa7296e3716695_8d2681 --label cbb281 --workdir /github/workspace --rm -e "INPUT_GITHUB_TOKEN" -e "INPUT_TFSEC_ARGS" -e "INPUT_WORKING_DIRECTORY" -e "INPUT_TFSEC_VERSION" -e "INPUT_TFSEC_FORMATS" -e "INPUT_COMMENTER_VERSION" -e "INPUT_SOFT_FAIL_COMMENTER" -e "HOME" -e "GITHUB_JOB" -e "GITHUB_REF" -e "GITHUB_SHA" -e "GITHUB_REPOSITORY" -e "GITHUB_REPOSITORY_OWNER" -e "GITHUB_RUN_ID" -e "GITHUB_RUN_NUMBER" -e "GITHUB_RETENTION_DAYS" -e "GITHUB_RUN_ATTEMPT" -e "GITHUB_ACTOR" -e "GITHUB_TRIGGERING_ACTOR" -e "GITHUB_WORKFLOW" -e "GITHUB_HEAD_REF" -e "GITHUB_BASE_REF" -e "GITHUB_EVENT_NAME" -e "GITHUB_SERVER_URL" -e "GITHUB_API_URL" -e "GITHUB_GRAPHQL_URL" -e "GITHUB_REF_NAME" -e "GITHUB_REF_PROTECTED" -e "GITHUB_REF_TYPE" -e "GITHUB_WORKSPACE" -e "GITHUB_EVENT_PATH" -e "GITHUB_PATH" -e "GITHUB_ENV" -e "GITHUB_STEP_SUMMARY" -e "GITHUB_STATE" -e "GITHUB_OUTPUT" -e "GITHUB_ACTION" -e "GITHUB_ACTION_REPOSITORY" -e "GITHUB_ACTION_REF" -e "RUNNER_OS" -e "RUNNER_ARCH" -e "RUNNER_NAME" -e "RUNNER_TOOL_CACHE" -e "RUNNER_TEMP" -e "RUNNER_WORKSPACE" -e "ACTIONS_RUNTIME_URL" -e "ACTIONS_RUNTIME_TOKEN" -e "ACTIONS_CACHE_URL" -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/github/_work/_temp/_github_home":"/github/home" -v "/home/github/_work/_temp/_github_workflow":"/github/workflow" -v "/home/github/_work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/github/_work/terraform/terraform":"/github/workspace" cbb281:2a0638619a79478ab2aa7296e3716695

tfsec --out=results.json --format=json --soft-fail --debug .

Based on this, it looks like there is an issue with my PR and the results.json file syncing up

// CommentNotValidError returned when the comment is for a file or line not in the pr
type CommentNotValidError struct {
	filepath string
	lineNo   int
}
@FooBartn
Copy link

FooBartn commented Dec 9, 2022

I noticed this recently as well. The worst part about it is that anyone could be having this issue and they will just assume everything is fine because the tfsec check passes and we trust it.

@trevorvoncannon
Copy link
Author

Looked into this a little more today and was able to pull out the error that is triggering the ignore statement:

func (e CommentNotValidError) Error() string {
return fmt.Sprintf("There is nothing to comment on at line [%d] in file [%s]", e.lineNo, e.filepath)

Here is the results.json file and the errors:

{
	"results": [
		{
			"rule_id": "AVD-AWS-0099",
			"long_id": "aws-ec2-add-description-to-security-group",
			"rule_description": "Missing description for security group.",
			"rule_provider": "aws",
			"rule_service": "ec2",
			"impact": "Descriptions provide context for the firewall rule reasons",
			"resolution": "Add descriptions for all security groups",
			"links": [
				"https://aquasecurity.github.io/tfsec/v1.28.1/checks/aws/ec2/add-description-to-security-group/",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule"
			],
			"description": "Security group explicitly uses the default description.",
			"severity": "LOW",
			"warning": false,
			"status": 0,
			"resource": "aws_security_group.terratest-sg",
			"location": {
				"filename": "/github/workspace/terratest-poc/main.tf",
				"start_line": 9,
				"end_line": 18
			}
		},
		{
			"rule_id": "AVD-AWS-0124",
			"long_id": "aws-ec2-add-description-to-security-group-rule",
			"rule_description": "Missing description for security group rule.",
			"rule_provider": "aws",
			"rule_service": "ec2",
			"impact": "Descriptions provide context for the firewall rule reasons",
			"resolution": "Add descriptions for all security groups rules",
			"links": [
				"https://aquasecurity.github.io/tfsec/v1.28.1/checks/aws/ec2/add-description-to-security-group-rule/",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group_rule"
			],
			"description": "Security group rule does not have a description.",
			"severity": "LOW",
			"warning": false,
			"status": 0,
			"resource": "aws_security_group.terratest-sg",
			"location": {
				"filename": "/github/workspace/terratest-poc/main.tf",
				"start_line": 12,
				"end_line": 17
			}
		},
		{
			"rule_id": "AVD-AWS-0131",
			"long_id": "aws-ec2-enable-at-rest-encryption",
			"rule_description": "Instance with unencrypted block device.",
			"rule_provider": "aws",
			"rule_service": "ec2",
			"impact": "The block device could be compromised and read from",
			"resolution": "Turn on encryption for all block devices",
			"links": [
				"https://aquasecurity.github.io/tfsec/v1.28.1/checks/aws/ec2/enable-at-rest-encryption/",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/instance#ebs-ephemeral-and-root-block-devices"
			],
			"description": "Root block device is not encrypted.",
			"severity": "HIGH",
			"warning": false,
			"status": 0,
			"resource": "aws_instance.terratest-ec2-instance",
			"location": {
				"filename": "/github/workspace/terratest-poc/main.tf",
				"start_line": 1,
				"end_line": 7
			}
		},
		{
			"rule_id": "AVD-AWS-0028",
			"long_id": "aws-ec2-enforce-http-token-imds",
			"rule_description": "aws_instance should activate session tokens for Instance Metadata Service.",
			"rule_provider": "aws",
			"rule_service": "ec2",
			"impact": "Instance metadata service can be interacted with freely",
			"resolution": "Enable HTTP token requirement for IMDS",
			"links": [
				"https://aquasecurity.github.io/tfsec/v1.28.1/checks/aws/ec2/enforce-http-token-imds/",
				"https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/instance#metadata-options"
			],
			"description": "Instance does not require IMDS access to require a token",
			"severity": "HIGH",
			"warning": false,
			"status": 0,
			"resource": "aws_instance.terratest-ec2-instance",
			"location": {
				"filename": "/github/workspace/terratest-poc/main.tf",
				"start_line": 1,
				"end_line": 7
			}
		}
	]
}

starting the github commenter
Working in repository terraform
Working in PR 1337
TFSec found 4 issues
Working in GITHUB_WORKSPACE /github/workspace/
Preparing comment for violation of rule aws-ec2-add-description-to-security-group in terratest-poc/terratest-poc/main.tf
There is nothing to comment on at line [9] in file [terratest-poc/terratest-poc/main.tf]
Preparing comment for violation of rule aws-ec2-add-description-to-security-group-rule in terratest-poc/terratest-poc/main.tf
There is nothing to comment on at line [12] in file [terratest-poc/terratest-poc/main.tf]
Preparing comment for violation of rule aws-ec2-enable-at-rest-encryption in terratest-poc/terratest-poc/main.tf
There is nothing to comment on at line [1] in file [terratest-poc/terratest-poc/main.tf]
Preparing comment for violation of rule aws-ec2-enforce-http-token-imds in terratest-poc/terratest-poc/main.tf
There is nothing to comment on at line [1] in file [terratest-poc/terratest-poc/main.tf]

@CharlesWinter
Copy link

I'm seeing the same error, and to my horror realised recently it's been happening silently for a while and I've just been assuming all is good with the sec scan!

@alex-bilimoria-quotient

+1

@moritzzimmer
Copy link

any updates on this?

@mario-fernandez-mm
Copy link

mario-fernandez-mm commented Jan 4, 2023

just add working_directory empty:

- name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@main
        with:
          working_directory: ''

This works for me!

@mandeeps13k
Copy link

I was also facing the same issue.

Thanks @mario-fernandez-mm , it seems that adding an empty working_directory does works.

@sysophost
Copy link

The suggested fix of adding a blank working_directory doesn't work when using --force-all-dirs.

@mandeeps13k
Copy link

@sysophost It seems to be working with --force-all-dirs with my setup. I already have --force-all-dirs in my workflow.

@sysophost
Copy link

@mandeeps13k I'm still getting the behaviour described by the OP.

Starting the github commenter
Working in repository my-repo
Working in PR 12
TFSec found 1 issues
Working in GITHUB_WORKSPACE /github/workspace/
Preparing comment for violation of rule custom-custom-001 in modules/security_group/rules.tf
Ignoring - change not part of the current PR

With this action definition

name: Run tfsec and comment on PR
on:
  - pull_request
jobs:
  tfsec:
    runs-on: ubuntu-latest

    permissions:
      contents: read
      pull-requests: write

    steps:
      - uses: actions/checkout@v3
        with:
          ref: ${{ github.event.pull_request.head.ref }}

      - name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@main
        with:
          github_token: ${{ github.token }}
          working_directory: ''
          tfsec_args: --concise-output --config-file=.tfsec.yml --custom-check-dir=.tfsec --force-all-dirs

.tfsec.yml contains a few entries in the exclude block. There is a single custom rule in .tfsec which is the rule that is being picked up for violation (custom-custom-001).

@ahmetrehaseker
Copy link

ahmetrehaseker commented Feb 23, 2023

I am seeing the same issue,
I checked the code and the problem is related with commenter. Commenter checks if the comment line fits the changed lines. If the changed file contains multiple changed locations (ie changes -> from line 3 to line 10 and line 20 to line 30) commenter only checks the first hunk. And the tfsec problem is in between line 20 and line 30 it throws the exception Ignoring - change not part of the current PR

If you fix this that would be awesome.

@saerosV
Copy link

saerosV commented Feb 27, 2023

@sysophost @ahmetrehaseker

What version of the tfsec-commenter-action are you guys using?

Try forcing the use of the 1.2.0 version, it might solve this problem.

A similar error was described here.

@sysophost
Copy link

Thanks for the suggestion @saerosV, I had tried pinning to an older version but was getting the same behaviour.
I think my issue comes from the fact I'm creating a resource from a module where the templated module is not being changed in the PR, just the place where it's instantiated.

Preparing comment for violation of rule custom-001 in modules/security_group/rules.tf
Ignoring - change not part of the current PR

In this case my tf is in a totally separate dir to the module, but as the resource is referencing the template in modules I assume it's being treated as a resource that is actually created in the modules dir rather than the dir where I'm creating an instance of the module.

@ahmetrehaseker
Copy link

@sysophost @ahmetrehaseker

What version of the tfsec-commenter-action are you guys using?

Try forcing the use of the 1.2.0 version, it might solve this problem.

A similar error was described here.

I did not test with that version but checked out the repo and debugged it myself, the problem is with the library you are using for commenting on the pr, as I wrote in the comment if you have multiple changed parts in the file library only checks for first hunk and ignores the other changes because of that return change is not part of the current PR
I created an issue for that repository Issue

@zachreborn
Copy link

Having the same this issue here. Certainly was working on these files in my PR.

Starting the github commenter
Working in repository terraform-modules
Working in PR 18
TFSec found 108 issues
Working in GITHUB_WORKSPACE /github/workspace/
Preparing comment for violation of rule aws-iam-no-policy-wildcards in ./global/iam/iam_groups/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-iam-no-policy-wildcards in ./global/iam/iam_groups/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-iam-no-policy-wildcards in ./global/iam/iam_groups/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-iam-no-policy-wildcards in ./global/iam/iam_groups/main.tf
Ignoring - change not part of the current PR
Preparing comment for violation of rule aws-iam-no-policy-wildcards in ./global/iam/iam_groups/main.tf
Ignoring - change not part of the current PR

My actions file:

- name: Run tfsec and write pull request comments
        uses: aquasecurity/tfsec-pr-commenter-action@main
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          tfsec_args: --force-all-dirs

@matheusjunior
Copy link

matheusjunior commented Apr 14, 2023

I am also having this issue on v1.3.1. My workflow:

name: tfsec
on:
  pull_request:
    paths:
        - 'terraform/modules/**'
  workflow_dispatch:
    inputs:
      environment:
        description: 'Environment'
        required: true
        default: 'dev'
        type: choice
        options:
          - dev
          - staging
          - preprod
      refToBuild:
        description: 'Branch, tag or commit SHA1 to build (default HEAD)'
        required: false
        default: ""
        type: string

jobs:
  tfsec:
    name: tfsec PR commenter
    runs-on: ubuntu-latest

    permissions:
      contents: read
      pull-requests: write

    steps:
      - name: Clone repo
        uses: actions/checkout@v3
      - name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@v1.3.1
        with:
          github_token: ${{ github.token }}
          working_directory: terraform/modules

I downgraded to v1.2.0 and still got this error.

@kaushal02
Copy link

+1

@c0nn0rstevens
Copy link

c0nn0rstevens commented May 23, 2023

I am also experiencing this issue, even when using the --no-ignores argument.

@kobylianskii
Copy link

+1

@carjessu-trm
Copy link

+1

@LeBaronDeCharlus
Copy link

+1

1 similar comment
@AhmedQaziMuhammadJamil
Copy link

+1

@cobrakainacho
Copy link

cobrakainacho commented Jun 21, 2023

similar issue here
I did changes to test it , it recognize the "error" in the change but say that it does not belong to PR , when it does.

`name: tfsec-pr-commenter
on:
  pull_request:

permissions:
  pull-requests: write
  actions: write
  contents: write
  id-token: write

jobs:
  tfsec:
    name: tfsec PR commenter
    runs-on: ubuntu-latest
    steps:
      - name: Clone repo
        uses: actions/checkout@v3
      - name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@v1.2.0
        with:
          github_token: ${{ secrets.GITHUB_TOKEN }}
          tfsec_args: --force-all-dirs`

@ZsoltPath ZsoltPath linked a pull request Jul 3, 2023 that will close this issue
@ohrafaelmartins
Copy link

just add working_directory empty:

- name: tfsec
        uses: aquasecurity/tfsec-pr-commenter-action@main
        with:
          working_directory: ''

This works for me!

Thank you so much. Its save me week

Stretch96 added a commit to dxw/terraform-dxw-dalmatian-account-bootstrap that referenced this issue Oct 17, 2023
* There is a bug in tfsec, where it finds vulnerabilities, but just
  outputs 'Ignoring - change not part of the current PR' - Even though
  it is. This is causing the check to pass, when it should fail.
* Adding `working_directory: ''` is a suggested fix
* aquasecurity/tfsec-pr-commenter-action#90 (comment)
Stretch96 added a commit to dxw/terraform-template that referenced this issue Oct 17, 2023
* There is a bug in tfsec, where it finds vulnerabilities, but just
  outputs 'Ignoring - change not part of the current PR' - Even though
  it is. This is causing the check to pass, when it should fail.
* Adding `working_directory: ''` fixes the issue
* aquasecurity/tfsec-pr-commenter-action#90 (comment)
Stretch96 added a commit to dxw/terraform-dxw-dalmatian-account-bootstrap that referenced this issue Oct 17, 2023
* There is a bug in tfsec, where it finds vulnerabilities, but just
  outputs 'Ignoring - change not part of the current PR' - Even though
  it is. This is causing the check to pass, when it should fail.
* Adding `working_directory: ''` is a suggested fix
* aquasecurity/tfsec-pr-commenter-action#90 (comment)
@GregUK
Copy link

GregUK commented Nov 7, 2023

Also hitting this issue. A fix has been suggested on the commenter action that is used but has not yet been merged owenrumney/go-github-pr-commenter#14

@ken5scal
Copy link

The repository go-github-pr-commenter has been archived :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.