aquasecurity / tfsec-pr-commenter-action

Add comments to pull requests where tfsec checks have failed
MIT License
164 stars 63 forks source link

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

Open trevorvoncannon opened 1 year ago

trevorvoncannon commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

https://github.com/aquasecurity/tfsec-pr-commenter-action/blob/7a44c5dcde5dfab737363e391800629e27b6376b/vendor/github.com/owenrumney/go-github-pr-commenter/commenter/errors.go#L50-L51

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 commented 1 year ago

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 commented 1 year ago

+1

moritzzimmer commented 1 year ago

any updates on this?

mario-fernandez-mm commented 1 year ago

just add working_directory empty:

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

This works for me!

mandeeps13k commented 1 year ago

I was also facing the same issue.

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

sysophost commented 1 year ago

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

mandeeps13k commented 1 year ago

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

sysophost commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

+1

c0nn0rstevens commented 1 year ago

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

kobylianskii commented 1 year ago

+1

carjessu-trm commented 1 year ago

+1

LeBaronDeCharlus commented 1 year ago

+1

AhmedQaziMuhammadJamil commented 1 year ago

+1

cobrakainacho commented 1 year ago

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`
ohrafaelmartins commented 1 year ago

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

GregUK commented 12 months ago

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

ken5scal commented 2 months ago

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