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

[BUG] Running with docker image generates empty baseline file #131

Open
cyrusfurtado opened this issue Jun 15, 2023 · 9 comments
Open

[BUG] Running with docker image generates empty baseline file #131

cyrusfurtado opened this issue Jun 15, 2023 · 9 comments

Comments

@cyrusfurtado
Copy link

cyrusfurtado commented Jun 15, 2023

Describe the bug
Running the docker image of the tool generates a .secrets.baseline file with an empty result for the secrets.

To reproduce
Steps to reproduce the behavior

  1. Switch to the directory where you need to run the tool
  2. Run detect-secrets scan --update .secrets.baseline --exclude-files="go.sum"
  3. Check the generated .secrets.baseline json file. The result property will have some entries.
"results": {
    "src/go/stream/stream.go": [
      {
        "hashed_secret": "524a8fbb68e4f36499a8300890f82558f64cc61d",
        "is_verified": false,
        "line_number": 76,
        "type": "Secret Keyword",
        "verified_result": null
      }
    ]
  },
  "version": "0.13.1+ibm.60.dss",
  1. Run docker run -it --rm -v $(pwd):/code icr.io/git-defenders/detect-secrets:redhat-ubi scan --update .secrets.baseline --exclude-files="go.sum"
  2. Check the updated .secrets.baseline json file. It will have an empty result property.
"results": {},
  "version": "0.13.1+ibm.61.dss",

Screenshots
Local tool run
secrets-local

Docker container run
secrets-container

Impact
High

Additional context:

  • Host or local: local
  • Operating System: Red Hat Enterprise Linux 8.7 (Ootpa)
  • Languages scanned: GoLang
  • Repo & build context links: N/A
  • Log output: N/A
@cyrusfurtado cyrusfurtado changed the title [Bug] Running with docker image generates empty baseline file [BUG] Running with docker image generates empty baseline file Jun 26, 2023
@victoria-miltcheva
Copy link
Member

victoria-miltcheva commented Jun 26, 2023

Hi @cyrusfurtado, the team is aware of this issue. In the meantime, the recommendation is to use the pip package as it does not have this empty baseline file issue: https://github.com/IBM/detect-secrets#installupgrade-module

@cyrusfurtado
Copy link
Author

@victoria-miltcheva thanks for the update. Since this is presently a known issue is there some timeline as to when the issue could be addressed.

@rjknight123
Copy link

Hi, I am hitting this issue also, any updates or workaround for the docker image?

@frederic-pesquet-work
Copy link
Member

I bumped into the same issue.
Digging more, it is due to a git error when detect-secrets run this line: https://github.com/IBM/detect-secrets/blob/master/detect_secrets/core/baseline.py#L366
Basically inside the docker, you are root and git complains:

$ docker run -ti --rm -v $(pwd):/code --entrypoint=bash icr.io/git-defenders/detect-secrets:latest
root@5d173a0cfc99:/code# git -C . ls-files
fatal: detected dubious ownership in repository at '/code'
To add an exception for this directory, call:
        git config --global --add safe.directory /code

=> as a result, none of the files are considered git files, and scan returns empty result.

Workarround:
You can run the docker as the current user with --user=$(id -u) option:

docker run --user=$(id -u) --pull=always -a stdout --rm -v $(pwd):/code icr.io/git-defenders/detect-secrets:redhat-ubi scan --update .secrets.baseline

@DenisMedeiros
Copy link

We were also facing this issue and @frederic-pesquet-work already figured out the cause, as he explained above.

Another finding: mounting volumes to docker running on Linux seems to work differently on Docker desktop (some colleagues using Docker Desktop on MacOS don't experience this bug).

When a volume is mounted in docker running on Linux, the file's ownership and permissions are kept "as is" inside of the container. For example, if the files in your $(pwd) are owned by uid 1000, then they show up inside of the container as owned by uid 1000, and then the git ls-files command fails (dubious ownership) because the detect-secrets container is running by default as root (uid 0).

On the other hand, in Docker Desktop the files mounted volume inside of the container already shows up owned by uid 0 so the git ls-files works fine.

@dleehr
Copy link

dleehr commented Nov 15, 2023

I have a couple thoughts.git ls-files will exit nonzero in this case (dubious permissions), which would raise an exception out of subprocess.check_output. There's logic to catch CalledProcessError but it just runs pass, so this case quietly continues.

I suspect there's a reason for that, but I'd be happy to submit a PR to change the behavior to fail (at least in some cases). I think it would be preferable to fail instead of generating an empty baseline file.

@rossgrady
Copy link
Member

@dleehr Oh so this loops back to the same thing we were talking about last week re: git being more picky nowadays re: dubious ownership.

Looking at the output above: fatal: detected dubious ownership in repository at '/code' it seems like even if, in a pipeline context, running the git config --global --add safe.directory pointing to the local clone wouldn't solve the problem, since it's mounted into the container at a different location (and presumably the git context within the container is different anyway)

But for the purposes of the detect-secrets documentation, it seems like it would be sufficient to modify the documentation so that it specifies running as the user, as @frederic-pesquet-work suggests, i.e.

docker run --user=$(id -u) --pull=always -a stdout --rm -v $(pwd):/code icr.io/git-defenders/detect-secrets:redhat-ubi scan --update .secrets.baseline

@dleehr
Copy link

dleehr commented Nov 15, 2023

@rossgrady Yep, I was reviewing the pipeline discussion yesterday and that's how I found may way here. I totally agree it's a good idea to update the documentation so that users can get valid results. I wasn't bringing pipeline questions here though :)

My suggestion here is that a failure in git ls-files should cause detect-secrets to fail. The current behavior is that when the git ls-files command fails (for the dubious permission error or any other error), that failure is ignored and _get_git_tracked_files returns an empty list []. That's clearly causing the empty baseline file (and the pipeline/container issues).

Now that we know about this, we can work around it (both in the docs and in pipeline). But it seems like it would be safer to fail rather than quietly succeeding with invalid results.

Looking at the output above: fatal: detected dubious ownership in repository at '/code' it seems like even if, in a pipeline context, running the git config --global --add safe.directory pointing to the local clone wouldn't solve the problem, since it's mounted into the container at a different location (and presumably the git context within the container is different anyway)

Yes, the safe.directory needs to be local to wherever detect-secrets is running. It's also only honored from $HOME/.gitconfig So in the pipeline context we need to account for that, and I have a PR for that. It sounds like in the docker run ... context here, the container entrypoint assumes that the repo is always mounted at /code. Providing guidance on making sure uids match can help, but since /code is expected to be a git repo and it it should be fair for git to trust it, adding a /root/.gitconfig to the image with

[safe]
	directory = /code

should fix the issue too.

@bigpick
Copy link
Member

bigpick commented May 6, 2024

Reading through this thread now, as just put a PR for #148 up (#150), which seems as a related to this issue (problems with git resulting in an empty baseline).

The PR still doesn't fail if it detects a git failure, though (but at least logs it now, so is more evident what went wrong if something fails)

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

8 participants