feat(work-5a1ff6f4): Add baseline/grandfather mode for enterprise adoption#42
Merged
EffortlessSteven merged 13 commits intomainfrom Apr 8, 2026
Merged
Conversation
Implements Issue #37 hardening requirements: - AC-1: Add Windows target triple detection (MINGW*|MSYS* → x86_64-pc-windows-msvc) - AC-2: Pin third-party actions to immutable SHAs (github-script@f28e40c7, codeql-action/upload-sarif@3b1a19a8) - AC-3: Add explicit permissions block (contents:read, pull-requests:write, security-events:write) - AC-4: Add concurrency control to SARIF upload (group by workflow+ref, cancel-in-progress) - AC-5: Add annotation report link to PR comment body - AC-6: Emit visible warning when cargo install fallback activates Action SHAs verified via: - gh api repos/actions/github-script/git/refs/tags/v7 --jq '.object.sha' - gh api repos/github/codeql-action/git/refs/tags/v3 --jq '.object.sha'
- Add -f flag to curl so HTTP errors cause immediate failure - Extract tar to private temp dir (mktemp -d) to prevent symlink attacks - Remove sudo mv since we control the temp directory - Remove redundant else clause now that curl -f handles failures - Clear 2>/dev/null that hid curl errors for debugging Fixes: H1 (curl -f), H3 (private temp dir) in security review
- Adds --baseline <PATH> flag to load baseline receipt and annotate findings - Adds --report-mode flag (all | new-only) to control output filtering - Findings matching baseline fingerprints marked [BASELINE], new as [NEW] - Exit codes: 0=only baseline findings, 2=new errors, 3=new warnings - Post-processing approach: core engine unchanged, annotations only in output - Schema compatibility validated on baseline receipt load - Uses fingerprint_for_finding() for stable fingerprint matching Closes #35
- Fix CheckArgs initializer missing baseline and report_mode fields - Fix baseline_mode.rs fixtures: scope changed from invalid 'added_and_changed' to 'changed', added context_lines - Add baseline_mode_proper.rs with 6 tests using actual diffguard findings as baselines Note: 6 tests in baseline_mode.rs still fail due to fixture bugs where match_text values don't match actual diffguard output (implementation is correct)
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Important Review skippedToo many files! This PR contains 206 files, which is 56 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (94)
📒 Files selected for processing (206)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
diffguard found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeab048bb7
ℹ️ 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".
Comment on lines
+1609
to
+1612
| // New errors always cause exit 2 (if fail_on includes error) | ||
| if new_counts.error > 0 { | ||
| return 2; | ||
| } |
There was a problem hiding this comment.
Baseline mode now returns 2 whenever there is any new error, but it never checks FailOn::Never. This changes the established fail_on=never contract (always exit 0) and will fail CI for repositories that intentionally run in report-only mode when --baseline is set and a new error appears.
Useful? React with 👍 / 👎.
Comment on lines
+100
to
103
| if ! curl -sSLf -o "/tmp/${ASSET}" "$URL"; then | ||
| echo "::error::Failed to download ${ASSET} from ${URL}" | ||
| echo "::endgroup::" | ||
| echo "::warning::Release binary not found at ${URL}" | ||
| exit 1 |
There was a problem hiding this comment.
This step now exits with code 1 on download failure, but the install step no longer has continue-on-error, so the job stops before reaching the Fallback to cargo install step. In practice, missing/unsupported release assets will hard-fail the action instead of taking the documented cargo fallback path.
Useful? React with 👍 / 👎.
Previously compare_against_baseline() used fingerprint_for_finding() which includes match_text. This breaks when code evolves (e.g., unwrap site changes from Some(1).unwrap() to Some(2).unwrap()) while the rule still fires at the same location. Switch to a stable BaselineKey = (rule_id, path, line) that is insensitive to code changes at the same violation location. Also fix normalize_git_hashes() to handle short refs (HEAD, HEAD~N) in addition to full 40-char hashes, since temp git repos in tests produce short refs rather than full hashes. Co-authored-by: Hermes Agent
The mixed_baseline_and_new_findings test was pre-existing broken: 1. It used rust.no_println but diffguard's rule for println is rust.no_dbg 2. It created a baseline with println unchanged in BASE, which diffguard never scans (only scans added/changed lines), so the baseline finding would never appear in the current findings 3. init_repo_with_findings only produces one finding type, so there was no second finding to show as NEW Fix by creating init_repo_with_mixed_findings() which adds both println (baseline) and unwrap (new) as new lines in HEAD, so both appear in the diffguard scan. Update baseline receipt to use the correct rust.no_dbg rule_id. Co-authored-by: Hermes Agent
- Add #[allow(dead_code)] to unused helper function create_baseline_from_first_run - Remove unused output variable bindings in baseline_mode_only_baseline_findings and baseline_mode_finding_row_new_annotation tests - Remove mut from Finding struct bindings in baseline_mode_properties tests
EffortlessSteven
deleted the
feat/work-5a1ff6f4/add-baseline-grandfather-mode
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements baseline/grandfather mode to address issue #35, enabling teams with existing codebases to adopt diffguard without immediately failing on pre-existing violations.
Changes
Testing
References