The pull request that passed clean because of a file name

What the check promises

PerspectiveGraph can analyze a pull request. It takes the scan of what the pull request builds, inserts it into an existing environment graph and asks whether this change opens a path from the internet to something sensitive. It doesn't fail on the number of vulnerabilities. A critical one passes if it's on a machine inaccessible to anyone; a medium-level one on a container now reaching the customer's database won't pass.

Most checks in a pipeline have two responses, and a scan that never arrived looks like a scan that found nothing. So this has a third response. clean means the engine analyzed the change and it isn't opening any paths. blocked means it's opening one. unknown means no one analyzed it - the scan, the upload or the commit was incorrect - and the build fails as if it were blocked.

How a pipeline scans an image

My sample reports all come from scanning an image by its name, which requires the image to be requested from a Docker daemon or a registry. A CI job often doesn't have either of these available. The usual method is to save the image to a file and scan the file:

docker save payments-api:1.0.0 -o /work/image.tar
trivy image --input /work/image.tar --format json --output trivy.json

Trivy scans it equally well. But look at how the report names what was scanned:

{
  "ArtifactName": "/work/image.tar",
  "ArtifactType": "container_image",
  "Metadata": {
    "Reference": "payments-api:1.0.0",
    "RepoTags": ["payments-api:1.0.0"]
  }
}

If scanned by name, ArtifactName would be payments-api:1.0.0. If scanned from the archive it's the path the file had on the build machine.

What the engine did with it

The engine connects a scan to the rest of the graph using the image name. The cluster says "this container runs payments-api:1.0.0"; the scan says "payments-api:1.0.0 contains this library, which has this vulnerability". The same name, one node, and a path can pass from the container through the image to the vulnerability and then to whatever exploiting it provides.

My reader interpreted ArtifactName as that name. So the scan created an image called /work/image.tar, with all its libraries and vulnerabilities attached, and no container anywhere runs an image with that name. Everything in the report was in the graph. None of it was linked to anything else.

Then the check asked its question. Was the commit analyzed? Yes: a scan marked with it arrived and was read. Does a path pass through what was changed? No: nothing leads to a disconnected node. Answer: clean, exit code 0, green checkmark. On a pull request whose image, once joined by its real name, is on a path from a public load balancer to a sensitive asset.

The third response wasn't helpful, because of how I'd defined it. unknown referred to a scan that didn't arrive. This one arrived. I had asked "did we look?" and treated the response as if it were "did we look at the right thing?".

How I found it

Not through a test, nor by reading the code. I was packaging the check as a plugin for Trivy, so it would run where the vulnerability list was already present. A plugin needs to be tested the way it will be used, so for the first time, I ran the entire chain with the real scanner on a real saved image, not with a report from my samples directory. The path I knew existed didn't come out.

before   CLEAN     exit 0
after    BLOCKED   exit 1

Every sample I had was a scan by name. My tests were correct about the reports I'd provided, and I'd given them the only type that worked by construction.

Running it this way revealed a second error. The check accepts -report - to read the report from standard input, which is how a Trivy plugin receives it. In one of its two modes, it opened a file literally named -, failed immediately, and exited. Trivy, stuck writing to a terminated process, didn't fail: it waited, indefinitely. A pipeline would hang until it timed out.

The fix

Trivy records the name under which the archive was saved, as shown in the previous report. The reader now uses it, under two rules.

• Only in place of a path. When ArtifactName is a file path - it starts with /, . or ~, contains a backslash, or ends in .tar, .tar.gz or .tgz - the image takes the saved name.
• Never in place of a name someone scanned by. Scanning nginx makes Trivy record the reference as nginx:latest. A workload that runs nginx must go on meeting the image it always met, so a name that is not a path is left alone.

The replaced information is written on the image node - the path it was scanned from, and the field where the name comes from - so the graph shows where a name comes from when someone asks why two items were merged.

The test runs the check on the same pull request twice: scanned by name, as a control, and scanned from an archive. Without the control, a check that simply stopped finding paths would have passed the test. With the fix removed, the archive case fails with the words "answered CLEAN" and the control continues to block.

What it costs the people using it

A fix like this turns green checkmarks red. Pull requests that passed the day before can fail the next day, without anything having changed. The upgrade note explicitly states this: they were passing because the scan had never been linked to the workload, not because the change was safe. I'd rather explain a red checkmark than leave a green one that means nothing.

What is still open, and what this does not prove

• An archive saved by image ID doesn't contain a name. The docker save command, if it receives an ID instead of a name, doesn't write any tag, and the report doesn't indicate which workload is running the image. The engine keeps it under its file path and writes the reason to the node. However, it isn't associated with any workload and the check still passes.
• The check can't distinguish between "no one is running this" and "I didn't recognize it". An image that no workload is running yet is a legitimate clean: a new service, not deployed, opens no path. From within the graph, the two cases look the same, so I can't turn the first into unknown without turning the second into it too.
• One scanner, one version. This is how Trivy 0.71 names an archive scan. The test reproduces the fields in the report; it doesn't run Trivy, so a later version that names things differently would not fail it.
• Other tools haven't been checked this way. Each source read by the engine has its own definition of what an element is called, and I've run only some of them the way a pipeline does. The same class of bug, found in several sources, is the subject of another article.

What I take from it

A third answer doesn't bridge the gap between "the data arrived" and "the data was linked". A check that says clean makes the second statement, while mine only checked the first.

The samples are written by the reader's author and are consistent with each other. The scanner, running as it does in a pipeline, has no such loyalty. A plugin's packaging found a flaw that the tests had ignored since the check was written.

And the worst result a gate can produce is not a bad block. A bad block is noisy and someone complains. A bad pass is silent and it is believed.

• trivy.go - how the image's name is chosen, and when the saved name is used
• gatelocal_test.go - the same pull request scanned both ways, with the control
• Upgrade notes for 1.18.0 - what changes for a pipeline that scans archives
• Attack paths in the pull request - the three answers, and what each one means