upload-sarif: relative paths beginning with `./` are not handled correctly
Chưa có ai nhận issue này.
Đánh giá
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức phù hợp với người mới
- 25/100
- Loại issue
- Lỗi
- Độ rõ ràng
- Khá rõ ràng
- Mức độ hoạt động
- Đình trệ
- Công nghệ
- github-actions
- Lĩnh vực
- security
Hướng nghiên cứu
Bắt đầu với entrypoint upload-sarif và tái hiện khác biệt đã được báo cáo giữa foo/bar và ./foo/bar, sử dụng hướng dẫn liên kết về đường dẫn SARIF và ngữ cảnh của RFC 3986. Điều tra xem phần triển khai liên quan nằm trong repository này hay trong cơ chế tiếp nhận SARIF của GitHub; được xem là hoàn tất khi các tham chiếu tương đối được phân giải thành các tệp trong repository, bản xem trước được render và xuất hiện nhất quán trên các pull request.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
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
- Ngôn ngữ chính
- TypeScript
- Star
- 1.6k
- Fork
- 493
- Merge trung bình
- 1 ngày 13 giờ
- Pull request đã merge (30 ngày)
- 44
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của github/codeql-action
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
github/codeql-action#4052 · 4 bình luận ·
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 45/100
github/codeql-action#4078 · 1 bình luận ·
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 48/100
github/codeql-action#4008 · 9 bình luận ·
-
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 52/100
github/codeql-action#3978 · 4 bình luận · 1 reaction ·
-
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 48/100
github/codeql-action#3915 · 6 bình luận · 3 reaction ·
Tất cả issue của github/codeql-action
Issue tương tự
-
[Bug]: Discord Activity titles with emoji are rejected as over 80 characters when they are not Đang mởclawsweeper:linked-pr-open clawsweeper:no-new-fix-pr clawsweeper:source-repro impact:message-loss issue-rating: 🦞 diamond lobster maturity:stable P2
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 88/100
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 84/100
Eynzof/Hermes-CN-Desktop#616 ·
-
ZCode 3.14.3 に対応する Đang mở
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 68/100
supermomonga/zcode-acp#24 ·
-
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 76/100
growthbook/growthbook#7100 ·
-
triage
Độ khó 1/5 1-3 giờ Mức phù hợp với người mới 88/100