upload-sarif: relative paths beginning with `./` are not handled correctly
まだ誰も着手していません。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 25/100
- issue の種類
- バグ
- 明瞭さ
- おおむね明確
- 活発さ
- 停滞
- 技術スタック
- github-actions
- 領域
- security
調査の方向性
upload-sarif エントリーポイントから始め、リンク先の SARIF パスに関するガイダンスと RFC 3986 の文脈を使って、foo/bar と ./foo/bar の報告された差異を再現します。関連する実装がこのリポジトリにあるのか、GitHub の SARIF 取り込み機構にあるのかを調査します。相対参照がリポジトリのファイルに解決され、プレビューが表示され、pull request に一貫して現れることが完了の条件です。
索引モデルが issue の本文から書いたものです。
説明
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.
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:
- Findings are not rendered on PRs, since GitHub doesn't think the finding URLs match repository paths;
- 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 ./:
For contrast, here's a similar finding rendering correctly because it doesn't begin with ./:
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:
- "Normalize" relative URIs from
./foo/bartofoo/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. - 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
- 主要言語
- TypeScript
- スター
- 1.6k
- フォーク
- 493
- 平均マージ
- 1日 13時間
- マージ済み PR(30日)
- 44
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
github/codeql-action のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
github/codeql-action#4052 · コメント 4 件 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 45/100
github/codeql-action#4078 · コメント 1 件 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
github/codeql-action#4008 · コメント 9 件 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 52/100
github/codeql-action#3978 · コメント 4 件 · リアクション 1 件 ·
-
難易度 3/5 1〜2日 初心者へのやさしさ 48/100
github/codeql-action#3915 · コメント 6 件 · リアクション 3 件 ·
github/codeql-action の issue をすべて見る
似ている issue
-
clawsweeper:linked-pr-open clawsweeper:no-new-fix-pr clawsweeper:source-repro impact:message-loss issue-rating: 🦞 diamond lobster maturity:stable P2
難易度 2/5 1〜3時間 初心者へのやさしさ 88/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
Eynzof/Hermes-CN-Desktop#616 ·
-
ZCode 3.14.3 に対応する オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
supermomonga/zcode-acp#24 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
growthbook/growthbook#7100 ·
-
triage
難易度 1/5 1〜3時間 初心者へのやさしさ 88/100