tfsec-pr-commenter-action icon indicating copy to clipboard operation
tfsec-pr-commenter-action copied to clipboard

.... not writing as not part of the current PR

Open cvs79 opened this issue 2 years ago • 11 comments

Integrated the commenter in my pipeline. It finds issues after some tweaking of the working directory.

But now it doenst write them to the PR as a comment. It keeps saying that the issue found is not part of the PR.

I've explicitly made an extea change to the file (keyvault.tf) in this case but that still results in the same logging output.

Any ideas?

cvs79 avatar Mar 17 '22 10:03 cvs79

I've been alerted to an issue where changes to variables don't show up in the commented PR. Does that sound like a similar scenario?

I'm guessing its not a publicly viewable repo?

owenrumney avatar Mar 23 '22 16:03 owenrumney

@owenrumney I think the issue is that when using the working_directory option, it fails to lookup the file: https://github.com/aquasecurity/tfsec-pr-commenter-action/blob/v1.2.0/cmd/commenter/commenter.go#L51-L56

If I understand it correctly, when providing a path to tfsec CLI, the output filename would be a relative path from that working_directory path. So it eventually hits this condition: https://github.com/aquasecurity/tfsec-pr-commenter-action/blob/1015a3975c7f1400ee4d9f423a7786a3df9fcbec/vendor/github.com/owenrumney/go-github-pr-commenter/commenter/commenter.go#L159 and that's where causing this issue, I guess.

smaeda-ks avatar Apr 18 '22 21:04 smaeda-ks

having the same issue here, fresh PR with files that are supposedly not good but are new as of this PR workflow:

1 file(s) written: results.json
Starting the github commenter...
Image scanning is not enabled. .... not writing as not part of the current PR
Repository tags are mutable. .... not writing as not part of the current PR

I've changed from using working_directory: terraform (my TF code is all in /terraform) to not having that key set at all - no difference.

Workflow:

      - name: Run TFsec
        uses: aquasecurity/[email protected]
        with:
          working_directory: terraform
          soft_fail_commenter: true
          tfsec_args: --soft-fail --concise-output --include-passed --minimum-severity MEDIUM
          github_token: ${{ github.token }}

These have the same problem:

with:
  commenter_version: v1.0.0
with:
  commenter_version: v0.1.10
with:
  commenter_version: v0.1.7

harmw avatar Apr 28 '22 19:04 harmw

I think this is due to a change in the tfsec filesystem scanning. the paths have changed so #63 might be the solution. I'll have a closer look

owenrumney avatar Apr 29 '22 09:04 owenrumney

Some debugging on a private repo with name ecr-provisioning:

declare -x GITHUB_WORKSPACE="/home/runner/work/ecr-provisioning/ecr-provisioning"

In this repo, I have terraform files in a subfolder ./terraform/ and tfsec found an issue in ecr.dummy.tf:

"location": {
  "filename": "/github/workspace/terraform/ecr.dummy.tf",
  "start_line": 3,
  "end_line": 3
}

However, it fails to process the comment:

Image scanning is not enabled. .... not writing as not part of the current PR

Looking at https://github.com/aquasecurity/tfsec-pr-commenter-action/blob/main/cmd/commenter/commenter.go#L51 it seems workspacePath is set to /home/runner/work/ecr-provisioning/ecr-provisioning/.

And some replacements done here at https://github.com/aquasecurity/tfsec-pr-commenter-action/blob/main/cmd/commenter/commenter.go#L54:

result.Range.Filename = strings.ReplaceAll(result.Range.Filename, workspacePath, "")

But filename in results.json is /github/workspace/terraform/ecr.dummy.tf whereas the above tries something on /home/runner/work/ecr-provisioning/ecr-provisioning/ 🤔

yet, the entrypoint.sh writes:

'[' -n /github/workspace ']'
+ cd /github/workspace

so even though I'm getting the above GITHUB_WORKSPACE in a separate workflow run step, the tfsec action step (running in docker) operates just fine? 🤔 🤦

harmw avatar Apr 29 '22 09:04 harmw

I found it a little painful to debug so tried adding some meaningful output in https://github.com/aquasecurity/tfsec-pr-commenter-action/pull/64 😅 it is difficult to reproduce though, between local docker and GitHub action 😞

🤦 🤦 🤦 seems my change actually wasn't part of the PR, just adding something new explicitly and behold the comments got added 🤦

harmw avatar Apr 29 '22 09:04 harmw

I have experienced this issue in my pipeline as well. I am making the changes directly in the directory that the action is supposed to be scanning, but for some reason, it still says that the error does not relate to the current PR. I am no sure what is causing this...

image

image

BMKeefer avatar Jul 21 '22 19:07 BMKeefer

I have been playing around with debugging this bad boy locally and found out that it points to the function that determines ( hardcoded for us ) if the file being commented on is relevant.

https://github.com/owenrumney/go-github-pr-commenter/blob/8aed49544a3f1352d15b059e0113a06182ac3bef/commenter/commenter.go#L155-L170

There might be more the the whole setup than just this - but I decided to quickly write up my own action using tfsec with JQ and other commenting framework 😎 as at the end I never got this one to run for me

RafPe avatar Sep 12 '22 08:09 RafPe

Hi, Is there any news on this ticket? I am using version 1.2.0 and the writing of comments in PR doesn't work, meaning I keep getting “not writing as not part of the current PR” message for all the security findings.

I have one PR with one commit and new resources created purposely to test the tool in this commit, in particular two s3 buckets without encryption.

When I compare changes in the PR I can clearly see the new S3 buckets resources defined for this only commit. But I still get the error.

I tried using parameters like working_directory for the action or adding the extra command --force-all-dirs but still, I am getting the message “not writing as not part of the current PR”.

@owenrumney

codingthecloud avatar Oct 25 '22 13:10 codingthecloud

Confirmed. The issue still exists and is really annoying, since it allows non-compliant resources to pass.

hi-artem avatar Nov 03 '22 17:11 hi-artem

Confirmed. The issue still exists

doomoscar avatar Nov 18 '22 22:11 doomoscar