upload-sarif: relative paths beginning with `./` are not handled correctly

Open
#2,829 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
github-actions
Domain
security

Research direction

Start with the upload-sarif entrypoint and reproduce the reported difference between foo/bar and ./foo/bar, using the linked SARIF path guidance and RFC 3986 context. Investigate whether the relevant implementation is in this repository or GitHub's SARIF ingestion machinery; done means relative references resolve to repository files, render previews, and appear on pull requests consistently.

Written by the indexing model from the issue text.

Description

Hello!

I'm reporting what I believe to be a bug in GitHub's SARIF consumption. This bug is probably happening somewhere deeper in GitHub's SARIF consumption machinery but upload-sarif is the main user-facing entrypoint for that machinery, so I'm filing the report here. Please let me know if a better discussion venue exists and I'd be happy to continue this elsewhere 🙂

Description

GitHub's documentation says the following about paths in SARIF inputs:

Code scanning interprets results that are reported with relative paths as relative to the root of the repository analyzed. If a result contains an absolute URI, the URI is converted to a relative URI. The relative URI can then be matched against a file committed to the repository.

Ref: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#specifying-the-root-for-source-files

This, combined with the fact that SARIF stipulates RFC 3986 for artifactLocation URIs suggests that relative paths should be able to include . and .., as these path components are well-defined in RFC 3986.

However, if a SARIF file contains a relative URI like ./foo/bar, GitHub appears to fail to resolve that URL to a path in the repository being scanned. This results in suboptimal finding presentation:

  1. Findings are not rendered on PRs, since GitHub doesn't think the finding URLs match repository paths;
  2. Findings are rendered under "Code scanning alerts" but show "Preview unavailable" instead of a proper synopsis/extraction of the source file.

Here is a screenshot of the above behavior, demonstrating the "Preview unavailable" behavior because the path (./tests/integration/test-data/issue-612-repro/action.yml) starts with a ./:

Image

For contrast, here's a similar finding rendering correctly because it doesn't begin with ./:

Image

Expected behavior

I expect GitHub's SARIF ingestion to handle URIs that begin with (or contain) relative references, since SARIF stipulates RFC 3986 for URIs and RFC 3986 permits relative references. More generally, I believe many code-scanning tools produce relative references (like ./foo) by default and would benefit from not having to specialize their handling for GitHub's SARIF consumer.

Actual behavior

Relative URIs like foo/bar work correctly (resolving relative to the repository root), while relative URIs like ./foo/bar do not work correctly.

Workarounds

The primary workarounds here are:

  1. "Normalize" relative URIs from ./foo/bar to foo/bar. This is possible, but non-trivial in the general case (e.g. ./foo/bar/../baz), and requires more pre-processing on the generated SARIF than the specification stipulates.
  2. Switch entirely to absolute URIs, and use SARIF's features (like invocations[0].workingDirectory.uri) to help GitHub ingest and transform paths into appropriate relative URIs. I've had limited success making this work: the SARIF ingestor appears to be very fickle about absolute paths.

Overall, I think both of these workarounds are non-ideal, and add additional burden to SARIF producers to pre-process their inputs/offer a GitHub specific "quirks mode" beyond what SARIF stipulates. I think it would be fantastic if GitHub could instead support these kinds of relative paths!

Additional context

I ran into this behavior within zizmor, which supports SARIF as an output format and encourages people to use it when integrating with GitHub. Some original issue context: https://github.com/woodruffw/zizmor/issues/604, https://github.com/woodruffw/zizmor/issues/571.

Additionally, I've observed that GitHub's behavior around equivalent relative paths (e.g. foo/bar and ./foo/bar) is somewhat mixed: ./foo/bar doesn't produce a preview, but results for foo/bar and ./foo/bar are deduplicated against each other. That makes me think that some degree of normalization/equivalence checking is happening in GitHub's SARIF processing, just not at the point needed to handle previews correctly.

Please let me know if there's any other information I can provide!

CC @kommendorkapten

Dominant language
TypeScript
Stars
1.6k
Forks
493
Avg merge
1d 13h
Merged PRs (30d)
44

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from github/codeql-action

All issues in github/codeql-action

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.