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

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

Open
cvs79 opened this issue Mar 17, 2022 · 11 comments
Open

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

cvs79 opened this issue Mar 17, 2022 · 11 comments

Comments

@cvs79
Copy link

cvs79 commented Mar 17, 2022

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?

@owenrumney
Copy link
Contributor

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?

@smaeda-ks
Copy link
Contributor

smaeda-ks commented Apr 18, 2022

@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:


and that's where causing this issue, I guess.

@harmw
Copy link
Contributor

harmw commented Apr 28, 2022

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/tfsec-pr-commenter-action@v1.2.0
        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

@owenrumney
Copy link
Contributor

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

@harmw
Copy link
Contributor

harmw commented Apr 29, 2022

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
Copy link
Contributor

harmw commented Apr 29, 2022

I found it a little painful to debug so tried adding some meaningful output in #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 🤦

@BMKeefer
Copy link

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

@RafPe
Copy link

RafPe commented Sep 12, 2022

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

@codingthecloud
Copy link

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

@hi-artem
Copy link

hi-artem commented Nov 3, 2022

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

@doomoscar
Copy link

Confirmed. The issue still exists

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

No branches or pull requests

9 participants