feat(action): SARIF → GitHub code scanning surface (dog-fooded in CI)#48
Conversation
Give the composite Action a fourth surface, `format: sarif`: it writes a
SARIF 2.1.0 log to a file and exposes the path as the `sarif-file` output, so
a consumer hands it straight to github/codeql-action/upload-sarif. Code
scanning subsumes the `github` PR-annotation format (it renders inline PR
annotations *and* the Security tab), so this stays a single own-check run.
An optional `sarif-file` input overrides the default $RUNNER_TEMP path. With
fail-on-finding the true exit code is captured *after* the file is written, so
the upload can still run (code scanning is the gate).
Validate the exporter on two levels — no "trust me bro":
- structure (own-check-surface): a jq contract over a freshly-emitted log —
version 2.1.0, the Own.NET driver, and every result carrying a catalogue
ruleId + a located file (the shape GitHub's ingest enforces). Read-only.
- end-to-end (own-check-codescan): the action `format: sarif` over the sample
tree, then upload-sarif under a *scoped* security-events:write (the only
non-contents:read job). upload-sarif waits for processing and fails the job
if GitHub rejects the log — live proof the SARIF is accepted, and it lights
up the Security tab on this public repo (the consumer payoff). Samples are
intentional fixtures, namespaced under the own-net-samples category.
docs/notes/sarif-export.md: flip the deferred code-scanning item to shipped
(the public-repo reasoning that reverses the earlier deferral) + the consumer
wiring snippet.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesSARIF Export Surface
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Workflow
participant Action as Own.NET composite action
participant Script as own-check.sh
participant GHOUT as GITHUB_OUTPUT
participant Upload as codeql-action/upload-sarif
Caller->>Action: format=sarif, severity=warning
Action->>Script: own-check.sh --format sarif --output <path>
Script-->>Action: writes SARIF file, returns rc
Action->>GHOUT: sarif-file=<path>
Action-->>Caller: outputs.sarif-file
Caller->>Caller: assert file non-empty
Caller->>Upload: sarif_file=<path>, category=own-net-samples
Upload-->>Caller: SARIF accepted by GitHub code scanning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36bedbd738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cat "$sarif" >&2 || true | ||
| exit "$rc" | ||
| fi | ||
| if [ "$OWN_FAIL_ON_FINDING" = "true" ] && [ "$rc" -eq 1 ]; then |
There was a problem hiding this comment.
Preserve non-check failures in SARIF mode
When format: sarif is used with the recommended fail-on-finding: false, this wrapper treats every exit status 1 as “findings only” and exits successfully. Failures outside own-check.sh's exit-code contract can also produce 1 here—for example, if the configured sarif-file parent directory does not exist, the shell redirection fails before own-check.sh runs—so consumers can get a successful action with an empty or missing SARIF output, and any later upload either fails elsewhere or is skipped. Check that the SARIF file was actually written, or fail redirection/setup errors separately, before applying the finding gate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 513-520: The `own-check-codescan` job requests security-events
write permission but runs unconditionally on pull_request events from forks,
where GitHub automatically downgrades permissions to read-only, causing the
upload-sarif step to fail. Add an `if` conditional guard to the
`own-check-codescan` job that prevents execution on fork pull requests by
checking that the event is not a pull_request OR that the pull request head
repository matches the target repository. This will prevent unnecessary failures
for external contributors while allowing the job to run on push events and pull
requests from the same repository.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c5af319-41c3-41d6-8d92-0aa9d016b607
📒 Files selected for processing (3)
.github/workflows/ci.ymlaction.ymldocs/notes/sarif-export.md
…review) Two review findings: - Codex (action.yml): in the sarif branch, exit 1 was treated as "findings, gated off -> exit 0", but a redirection/setup failure (e.g. a custom sarif-file whose parent dir is absent fails the `> "$sarif"` redirection before own-check.sh runs) also exits 1 — so the action could "succeed" with no SARIF. Add a non-empty-file guard below the hard-error tier: a missing or empty log with rc<2 is surfaced as an error, never gated as a finding. (Also make the byte-count echo tolerate an absent file under set -e.) Verified the five paths locally: clean/findings(gate off|on)/hard-error/missing-parent-dir. - CodeRabbit (ci.yml): own-check-codescan requests security-events:write but ran on fork PRs too, where GitHub downgrades the token to read-only -> upload-sarif fails -> persistent red CI for external contributors. Guard the job to same-repo events (push or non-fork PR) with the canonical `head.repo.full_name == github.repository` if-condition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
What
Give the composite Action a fourth surface,
format: sarif: it writes a SARIF 2.1.0 log to a file and exposes the path as thesarif-fileoutput, so a consumer hands it straight togithub/codeql-action/upload-sarif. Code scanning subsumes thegithubPR-annotation format — it renders inline PR annotations and the Security tab — so this stays a single own-check run, not two. An optionalsarif-fileinput overrides the default$RUNNER_TEMPpath.Consumer wiring is three steps:
This retires the need for our bespoke output on the consumer surface: code scanning draws both the inline PR annotations and the Security tab natively.
Validated in CI — two levels, no "trust me bro"
own-check-surface): ajqcontract over a freshly-emitted log —version 2.1.0, theOwn.NETdriver, and every result carrying a catalogueruleId+ a located file (the shape GitHub's ingest enforces). Read-only, no permissions. Locally cross-checked against 9 real core SARIF outputs (incl.unresolved→ OWN001+OWN050 anddi→ DI001).own-check-codescan): the actionformat: sarifover the sample tree, thenupload-sarifunder a scopedsecurity-events: write(the only non-contents:readjob).upload-sarifwaits for processing and fails the job if GitHub rejects the log — so a green run is live proof GitHub accepts our SARIF. The sample alerts ride a dedicatedown-net-samplescategory. ✅ green on this branch's run (the Security tab is now lit with the sample findings).Why dog-food here (the deferral reversed)
The earlier
sarif-export.mdnote deferred this as "low-value — the repo's C# is test fixtures." That weighed the findings; it mis-weighed the integration. The repo is public, so code scanning is free, and a live upload is the one thing a local schema check cannot give — proof of acceptance. The note is updated accordingly.Files
action.yml—format: sarifbranch (write SARIF to file, exposesarif-fileoutput; capture the true exit code after writing so the upload can still run); newsarif-fileinput..github/workflows/ci.yml— thejqcontract step + theown-check-codescandog-food job (scopedsecurity-events: write).docs/notes/sarif-export.md— deferred → shipped, with the consumer snippet + the public-repo reasoning.🤖 Generated with Claude Code
https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
Generated by Claude Code
Summary by CodeRabbit
New Features
sarifformat output to generate SARIF 2.1.0 logs suitable for GitHub code scanning.sarif-filepath for direct SARIF upload usage.Tests / CI
Documentation