Repository navigation
OSAC-1684: Add scan-workflow-logs listener - #393
openshift-merge-bot[bot] merged 2 commits into
Conversation
|
@minmzzhang: This pull request references OSAC-1684 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a GitHub Actions workflow triggered after selected E2E runs. It scans and purges logs, optionally comments on the matching pull request, retrieves a Slack webhook from Vault when needed, and sends leak or failure notifications. ChangesE2E log monitoring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UpstreamE2E
participant ScanWorkflow
participant ScanAndPurgeLogs
participant GitHubPRAPI
participant Vault
participant NotifySlack
UpstreamE2E->>ScanWorkflow: workflow_run completion
ScanWorkflow->>ScanAndPurgeLogs: scan and purge run logs
ScanAndPurgeLogs-->>ScanWorkflow: scan and purge results
ScanWorkflow->>GitHubPRAPI: resolve PR and post findings when leaks are found
ScanWorkflow->>Vault: retrieve Slack webhook when alerting
Vault-->>ScanWorkflow: Slack webhook URL
ScanWorkflow->>NotifySlack: send alert with outcome and run metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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/scan-e2e-logs.yml:
- Line 48: Replace the floating `@main` references for scan-and-purge-logs and
notify-slack with the full commit SHA containing the required
osac-test-infra#262 changes. Keep both cross-repository composite actions pinned
to that specific SHA, then update to the tagged release SHA once the change is
merged.
🪄 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: Enterprise
Run ID: ec5d38c9-cb96-460e-b0fe-d343d14b7d76
📒 Files selected for processing (1)
.github/workflows/scan-e2e-logs.yml
5abb9bc to
258697f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/scan-e2e-logs.yml (1)
104-145: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape Markdown syntax in sanitized finding cells.
The transforms escape HTML and
|, but leave link/code/backslash syntax active. A value such as`[login](https://attacker.example)`can render as a clickable link, and a backslash before|can defeat table escaping. Escape Markdown metacharacters and backslashes in both transforms, ideally through one tested shared helper.🤖 Prompt for 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. In @.github/workflows/scan-e2e-logs.yml around lines 104 - 145, Update the md_cell helper and the jq findings cell helper used when generating the PR comment to escape Markdown metacharacters and backslashes in addition to HTML characters and pipe delimiters. Ensure values such as links, code spans, and backslash-prefixed pipes render as inert table text, and consolidate the escaping logic through one tested shared helper where the workflow supports it.
🤖 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.
Outside diff comments:
In @.github/workflows/scan-e2e-logs.yml:
- Around line 104-145: Update the md_cell helper and the jq findings cell helper
used when generating the PR comment to escape Markdown metacharacters and
backslashes in addition to HTML characters and pipe delimiters. Ensure values
such as links, code spans, and backslash-prefixed pipes render as inert table
text, and consolidate the escaping logic through one tested shared helper where
the workflow supports it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d3a2ac01-fdf5-46e4-b041-5af46aed266e
📒 Files selected for processing (1)
.github/workflows/scan-e2e-logs.yml
|
/retest |
|
Re-triggered failed runs:
|
017241d to
4eee046
Compare
Mirror osac-test-infra scan-workflow-logs via cross-repo composite actions. Listen for VMaaS/BMaaS Full Install. Comment sanitized findings on the triggering PR; Slack warns on purged credential-only findings. Hardening: SHA-pin actions, timeout, shared should-alert gate, bash Slack payload, vault/notify/AppRole success gates, ::error::/summary fallback, fail-fast AppRole reads, and zizmor dangerous-triggers suppress (no PR-head checkout). Assisted-by: Cursor <noreply@cursor.com>
4eee046 to
79f4202
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz, minmzzhang, omer-vishlitzky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
osac-test-infrascan-workflow-logslistener via the cross-repo composite action.timeout-minutes: 15.Dependencies
osac-test-inframain. After osac-test-infra#262 merges, bump the pin to that merge commit so artifact purge is picked up.Test plan
Summary by CodeRabbit