Repository navigation
fix: preserve one-character directories in SARIF paths - #84
Conversation
Match only a literal leading ./ when normalizing SARIF artifact URIs. The unescaped dot also matched single-character directory prefixes and changed k/pod.yaml to pod.yaml. Exercise the full entrypoint with relative, workspace-absolute and file URI fixtures, including longer-directory and empty-location controls. Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: preserving one-character directory prefixes in SARIF paths. Linked Issues check Issue [ #83] requires removal of only a literal leading./from SARIF URIs.entrypoint.shchangessub("^./"; "")tosub("^\\\\./"; ""). The new entrypoint test coversk/pod.yaml, longer directo…Out of Scope Changes check The pull request changes only the SARIF normalization expression and the entrypoint test harness and regression fixture. The test harness changes support execution of the linked SARIF regression test.… Full details: Linked Issues check
-
Explanation
Issue [
#83] requires removal of only a literal leading./from SARIF URIs.entrypoint.shchangessub("^./"; "")tosub("^\\./"; ""). The new entrypoint test coversk/pod.yaml, longer directories,./paths, workspace-absolute paths,file://URIs, and empty-URI filtering. The expected output preservesk/pod.yamland removes only the intended prefixes.
Full details: Out of Scope Changes check
-
Explanation
The pull request changes only the SARIF normalization expression and the entrypoint test harness and regression fixture. The test harness changes support execution of the linked SARIF regression test. No unrelated product behavior, inputs, dependencies, or schema changes are present in the reviewed diff.
-
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
matthyx
left a comment
matthyx
left a comment
There was a problem hiding this comment.
Approve: the fix is needed and sound. No blocking findings in the two changed files.
Reviewed head 32b24fbbc8339790523474bfca692767291ea596 against main at 55308b8442c97f510a87f81f4b9d11d3e5419014. The unescaped regex dot removes k/ from k/pod.yaml; matching a literal dot repairs the cause while retaining existing prefix removal and empty-location filtering. No inputs, dependencies, schema, or resource handling change.
Validation in a credential-free, network-isolated Bubblewrap environment with read-only source and temporary writable storage:
bash tests/entrypoint_test.sh: 19 passed; the same tests with main's entrypoint: 18 passed, with only the new SARIF regression failing.bash tests/action_test.sh: 22 passed (after fixing an isolation-only missing awk symlink).bash -nfor entrypoint and both test scripts;git diff --check: passed.
The fixture covers relative paths, longer directory controls, literal ./, workspace-absolute paths, file URIs, and empty URI filtering. Full scanner/container execution and successful hosted Code Scanning upload were not independently validated; ShellCheck was unavailable.
History: #83 describes this exact defect. Merged #18 and #22 establish SARIF output/upload support; neither fixes this regex. #71 and #50 concern empty reports (the latter tracks Helm source attribution), not directory corruption. Closed #7 was explicitly replaced by #9; its input-validation discussion does not reject this normalization. Closed #44 has no explicit maintainer rejection of this approach. Main still contains the defect. No duplicate or superseding fix found.
Search scope: all repository PR/issue listings fit within the 100-item API pages; searched title/body/comment index for sarif, entrypoint.sh, and path, and inspected entrypoint history and related discussions. This is bounded repository-local evidence, not proof of absence across forks or unindexed discussions.
Head CI's entrypoint job passed. Scan/image jobs fail opening SARIF with EACCES and also report deprecated CodeQL v2; autofix artifact upload fails opening results.json with EACCES. These are separate permission/workflow problems: this diff changes neither file modes nor JSON handling. No existing human reviews or inline threads require resolution; CodeRabbit reports no actionable findings. Independent code review recommends approval; architecture assessment is clear.
PR description
Fixes #83
The SARIF normalizer uses
sub("^./"; ""), where the unescaped dot matches any character. This incorrectly changes legitimate paths such ask/pod.yamltopod.yaml, causing findings to reference the wrong file.This change:
./is removed.file://URIs, longer-directory negative controls, and existing empty-URI filtering.No action inputs, dependencies, or SARIF schema changes are introduced.
Validation
git diff --check: passed.pod.yaml, while the corrected action preservesk/pod.yaml.Hosted GitHub Actions execution and Code Scanning upload have not been verified.
Summary by CodeRabbit
./is recognized and stripped literally. Relative directories are preserved, supported path prefixes are removed, and empty URIs are omitted. This improves the consistency of file locations shown in SARIF output.