Repository navigation
NO-ISSUE: Pin osac-test-infra refs and add SAST/SCA gates - #16
Conversation
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: osac-project/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughThe PR adds CodeQL and dependency-review workflows for pull requests targeting ChangesCI security and workflow control
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
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. |
62d9077 to
9260f1e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/e2e-bmaas-full-install.yml:
- Around line 61-62: Make the external osac-test-infra reference immutable
before checkout: update test-infra-ref to a fixed commit or immutable tag in
.github/workflows/e2e-bmaas-full-install.yml:78 and
.github/workflows/e2e-vmaas-full-install.yml:83, and ensure the reusable
workflow invocations at .github/workflows/e2e-bmaas-full-install.yml:61-62 and
.github/workflows/e2e-vmaas-full-install.yml:63-64 use that pinned value rather
than mutable main.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ecdecede-5781-4c3a-b599-c9a59f24f6bb
📒 Files selected for processing (4)
.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/e2e-bmaas-full-install.yml.github/workflows/e2e-vmaas-full-install.yml
|
Reviewed the diff against a fresh checkout of current 1. This repo's (Building from inside CodeQL's Go autobuilder does effectively this from the checkout root when Suggested fix: don't rely on 2. The PR's stated deferral of The description says: "e2e-caas-full-install.yml doesn't exist on main yet -- it only exists on the still-open #14 branch -- so it isn't touched here. It'll get the same pin naturally once #14 merges." #14 has since merged ( Nothing else stood out — the dependency-review and CodeQL workflow structure/permissions otherwise look reasonable, and the judgment call to not hard-fail CodeQL on findings is well-reasoned and clearly flagged. |
Pin the osac-test-infra reusable-workflow references in e2e-bmaas-full-install.yml, e2e-vmaas-full-install.yml, and e2e-caas-full-install.yml from @main to the current osac-test-infra main commit SHA (5347598040856b07c61afed01b5b1e6f428c81d7, verified against osac-test-infra's main HEAD), with a comment noting the SHA needs to be bumped manually when osac-test-infra changes are needed. Add two previously-missing security gates, both new pull_request workflows pinned to full commit SHAs: - dependency-review.yml: actions/dependency-review-action (SCA), which fails the check when a PR introduces a new dependency with a known vulnerability. - codeql.yml: github/codeql-action (SAST) covering this repo's Go (fulfillment-service, osac-operator) and Python (fulfillment-service dev tooling) code. Scans and uploads findings to the Security tab rather than hard-failing on any finding count > 0 -- the gate is the analyze step completing successfully; failing on findings outright would turn every future PR red until the pre-existing finding backlog is triaged, which is a much bigger policy change than adding a scanning gate. The Go leg of codeql.yml failed its own check twice over: - The runner's preinstalled Go (1.24.13) is older than what root go.work requires (>= 1.26.3) for the fulfillment-service/osac-operator/osac-operator-api workspace. Fixed by running this repo's ./.github/actions/setup-go composite action (which installs exactly 1.26.3) before CodeQL's init/build steps on the go leg, matching how the rest of this repo's CI sets up Go rather than adding a raw actions/setup-go step. - autobuild runs `go build ./...` from the repo root, which go.work rejects ("directory prefix . does not contain modules listed in go.work or their selected dependencies") since there's no go.mod at root, only the workspace file. Switched the go leg to build-mode: manual with explicit per-module build steps (cd into fulfillment-service, osac-operator, and osac-operator/api and go build ./... in each), the same working-directory-scoping idea as the setup-go step.
9260f1e to
accb2ad
Compare
|
Both findings confirmed and fixed, pushed as an amended/rebased commit:
Kept this as a single amended commit per this PR's existing convention. Re-validated YAML parsing and |
|
Final verification pass on the two fixes from the earlier review, plus a drift check now that #28/#32/etc. have landed since. Both fixes confirmed solid. 1. CodeQL Go autobuild fix — confirmed. 2. caas SHA pin — confirmed, and checked for drift. Drift/conflict check against recently-landed PRs. Confirmed good to merge from my side. |
This branch was previously deployed
2 participants
Summary
Fixes the 2 pre-existing repo-wide gaps CodeRabbit flagged on #14 (branch
split-e2e-workflows-with-schedules), which were deliberately deferred there as out of scope for that PR. Branched offupstream/main, independent of #14.1. Pin osac-test-infra reusable-workflow references from
@mainto a commit SHAe2e-bmaas-full-install.ymlande2e-vmaas-full-install.ymlreferencedosac-project/osac-test-infra/.github/workflows/e2e-{bmaas,vmaas}-full-install.yml@main. Pinned both to5347598040856b07c61afed01b5b1e6f428c81d7, verified as osac-test-infra's currentmainHEAD viagh api repos/osac-project/osac-test-infra/commits/main --jq .shaimmediately before pinning. Added a one-line comment above each (# bump this SHA when osac-test-infra reusable workflows change) since the trade-off of pinning is that this repo won't pick up new osac-test-infra fixes/features until someone manually bumps it.e2e-caas-full-install.ymldoesn't exist onmainyet -- it only exists on the still-open #14 branch -- so it isn't touched here. It'll get the same pin naturally once #14 merges (or can be pinned directly on that branch if preferred).2. Add a pinned SAST/SCA gate
Verified there was no SAST or SCA/dependency-scanning workflow anywhere in
.github/workflows/before this change. Added two new pull-request-triggered workflows, both pinned to full commit SHAs:.github/workflows/dependency-review.yml--actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294(v5.0.0), plusactions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1(v7.0.1, matching the SHA already pinned elsewhere in this repo). Fails the PR check when it introduces a new dependency with a known vulnerability -- the SCA gate..github/workflows/codeql.yml--github/codeql-action/{init,autobuild,analyze}@f205ea1c3313d32999d8d6a48b4f6530d4437b38(v4.37.4), matrixed overgo(fulfillment-service, osac-operator) andpython(fulfillment-service dev tooling).Judgment call on CodeQL fail-on-findings behavior: the original ask was to "fail the pipeline when findings are detected." I did not implement that literally. CodeQL's standard behavior is to scan and upload results to the repo's Security tab rather than fail the run on findings; hard-failing on any finding count > 0 would make this new workflow immediately red on every future PR until someone triages whatever pre-existing findings already exist in this codebase -- a much larger, noisier policy change than "add a scanning gate," and one this PR shouldn't sneak in as a side effect. I went with standard CodeQL behavior (scan + report to Security tab), with the "gate" being the
analyzejob itself completing successfully (so build/scan tooling failures still fail the check). A defensible middle ground if stricter enforcement is wanted later: use CodeQL's severity-filtering query options to fail only onerror/critical-level findings once the existing backlog (if any) has been triaged -- happy to follow up with that once someone's looked at the Security tab post-merge.Validation
yaml.safe_load).pre-commit runon the changed files passes (trailing-whitespace, yamllint, etc.).Test plan
dependency-reviewandcodeqlon this PR and both complete without tooling errorse2e-bmaas-full-install/e2e-vmaas-full-installstill resolve and run correctly against the pinned osac-test-infra SHASummary by CodeRabbit