Skip to content

Commit

Permalink
fix: external tools producing duplicate keys
Browse files Browse the repository at this point in the history
Some tools work on context directories (e.g. sbom) and for non-fs
artifacts (e.g. docker image), sbom tool would run both during host
chalk time as well as artifact chalk time collections. As both would
collect its output for the same path input, it would produce duplicate
output in both report and chalk mark levels.

Now any tool can run at most once and any future attempts for the same
path are skipped which removes duplicate keys.
  • Loading branch information
miki725 committed Nov 5, 2024
1 parent eab26f0 commit 46dd014
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
- Docker pushing non-chalked images did not report metsys
plugin keys such as `_EXIT_CODE`, `_CHALK_RUN_TIME`.
([#438](https://github.com/crashappsec/chalk/pull/438))
- External tools for non-file artifacts (e.g. docker image)
sent duplicate keys in both report-level as well as
chalk-mark level. For example `SBOM` key with equivalent
content was duplicated twice.
([#440](https://github.com/crashappsec/chalk/pull/440))

### New Features

Expand Down
18 changes: 11 additions & 7 deletions src/plugins/externalTool.nim
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@
## This plugin uses information from the config file to set metadata
## keys.

import std/[algorithm, sequtils]
import std/[algorithm, sequtils, sets]
import ".."/[config, plugin_api, util]

type PIInfo = ref object
name: string
type
AlreadyRanError = object of CatchableError
PIInfo = ref object
name: string

proc ensureRunCallback[T](cb: CallbackObj, args: seq[Box]): T =
let value = runCallback(cb, args)
if value.isNone():
raise newException(ValueError, "missing implemenetation of " & $(cb))
return unpack[T](value.get())

var toolCache = initTable[string, ChalkDict]()
var alreadyRan = initHashSet[string]()
proc runOneTool(info: PIInfo, path: string): ChalkDict =
let key = info.name & ":" & path
if key in toolCache:
return toolCache[key]
if key in alreadyRan:
raise newException(AlreadyRanError, "")

trace("Running tool: " & info.name)
result = ChalkDict()
Expand Down Expand Up @@ -65,7 +67,7 @@ proc runOneTool(info: PIInfo, path: string): ChalkDict =
d.del("info")

trace(info.name & ": produced keys " & $(d.keys().toSeq()))
toolCache[key] = d
alreadyRan.incl(key)
return d

template toolBase(path: string) {.dirty.} =
Expand Down Expand Up @@ -109,6 +111,8 @@ template toolBase(path: string) {.dirty.} =
result.merge(data.nestWith(info.name))
if len(data) >= 0 and attrGet[bool]("tool." & info.name & ".stop_on_success"):
break
except AlreadyRanError:
trace(info.name & ": already ran for " & path & ". skipping")
except:
error(info.name & ": " & getCurrentExceptionMsg())

Expand Down
5 changes: 4 additions & 1 deletion tests/functional/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
validate_virtual_chalk,
)
from .conf import CODEOWNERS, CONFIGS, DATA, DOCKERFILES, LS_PATH, PYS
from .utils.dict import ANY
from .utils.dict import ANY, MISSING
from .utils.docker import Docker
from .utils.git import Git
from .utils.log import get_logger
Expand Down Expand Up @@ -869,6 +869,9 @@ def test_syft_docker(chalk_copy: Chalk, test_file: str, random_hex: str):
tag=tag,
)

assert build.report.contains(sbom_data)
assert build.mark.has(SBOM=MISSING)

# artifact is the docker image
artifact_info = ArtifactInfo(
type="Docker Image",
Expand Down

0 comments on commit 46dd014

Please # to comment.