anchore / anchore/syft

Paths that could not be accessed are recorded in errPaths and never reach the SBOM

Open
#5,153 1 comment 0 reactions 1 assignee Claimed by @kzantow View on GitHub
Dominant language
Go
Stars
9.6k
Forks
954
Avg merge
23h 27m
Merged PRs (30d)
48

Description

I do reliability work on the class of bug where a system reports an outcome it never confirmed, and
I went looking in Syft because an SBOM is a verdict about somebody else's software — a package that
could not be read becomes an *absence*, and absence is what a downstream scanner reads as "not
installed."

Filing this because you already have the right mechanism in the tree. One layer just does not
participate in it.

### The shape

Both indexers keep a map of every path they could not access:

`syft/internal/fileresolver/directory_indexer.go`
```go
// :272
r.errPaths[givenPath] = fmt.Errorf("no file info observable at path=%q", givenPath)

// :296-300
func (r *directoryIndexer) isFileAccessErr(path string, err error) bool {
// don't allow for errors to stop indexing, keep track of the paths and continue.
if err != nil {
log.Warnf("unable to access path=%q: %+v", path, err)
r.errPaths[path] = err
```

`syft/internal/fileresolver/file_indexer.go:221` does the same.

**Nothing reads either map.** Grepping `errPaths` across the repository returns three files: the two
indexers that write it, and `directory_indexer_test.go`, which asserts only that the map gets
*populated* (`_, exists := r.errPaths[p]`) — never that anything consumes it.

So the record exists, is accurate, and terminates. The `log.Warnf` goes to a log; the SBOM — the
artifact decisions are actually made on — carries nothing.

### Why I think this is an oversight rather than a policy

Because Syft already expresses exactly this concept, deliberately, one component over.
`internal/task/unknowns_tasks.go` maintains `s.Artifacts.Unknowns`, and two of its three default
options *add* entries for coverage gaps nobody explicitly asked about:

```go
// :51-57
if c.IncludeExecutablesWithoutPackages {
... append(..., "no package identified in executable file")
}
// :59-66
if c.IncludeUnexpandedArchives {
... append(..., "archive not cataloged")
}
```

That is the same instinct — *tell the consumer where we could not see* — implemented properly and on
by default. A file the indexer could never open is arguably a stronger unknown than an archive that
merely was not expanded, and it is the one case that does not arrive.

### Your own note, which is what made me look

`file_indexer.go:144-149`:

```go
// If we hit file access errors, isFileAccessErr will handle logging & adding
// the path to the errPaths map.
// While the directory_indexer does not let these cause the indexer to throw
// we will here, as not having access to the file we index for a file source
// probably makes the file source creation useless? I need to check with Syft maintainers.
// This also poses the question, is errPaths worthwhile for file_indexer?
```

Someone had already half-noticed. I think the answer to the question in that comment is that
`errPaths` is worthwhile in *both* indexers, and it is currently worthwhile in neither, because it
has no consumer.

### Why it matters more here than in most codebases

A directory scan that hits permission errors on part of the tree produces an SBOM that is
structurally indistinguishable from a complete one. Same shape, same schema, fewer packages, no
field saying so. Whatever consumes it — grype, a policy gate, a compliance attestation — reads a
short inventory as an accurate one, and a clean vulnerability report is exactly the answer nobody
re-examines.

`log.Warnf` is the right severity and the wrong channel: in CI that line is one of thousands, and
nothing downstream of the SBOM can see it at all.

### What I have not verified, plainly

I read this on `main` through the API and re-verified the line numbers the hour I filed. **I have
not run Syft and have not reproduced a partial scan.** Specifically I have not checked whether
`errPaths` is surfaced through some path GitHub code search does not index, or whether the
`file.Resolver` interface exposes it to callers under another name — if it does, this is much
narrower than it looks and I would like to know.

I killed three of my own findings this same week by tracing one layer further and discovering the
thing was already handled underneath, including one in this repository an hour ago.

### A possible shape for the fix

The cheapest version reuses what exists: have the indexers hand their `errPaths` to the unknowns
labeler so inaccessible paths land in `s.Artifacts.Unknowns` alongside the unexpanded archives —
same mechanism, same default, one more source. That also answers the open question in the comment
above, in the affirmative, for both indexers.

Happy to open a PR if the direction is welcome.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.