OAPE-906: add automated CVE triage tool for helm-operator - #459
OAPE-906: add automated CVE triage tool for helm-operator#459mytreya-rh wants to merge 1 commit into
Conversation
Adds a reusable govulncheck + VTA callgraph workflow that proves a Go binary is not affected by a CVE and closes the corresponding OCPBUGS Jira ticket with full evidence, plus a helm-operator driver wired up for the ose-helm-operator / ose-helm-rhel9-operator PS components. The library validates all Jira/OSV/vuln.go.dev requests against a host allowlist, keeps Jira credentials on a separate object from the component config used to build local paths, pins git refs to a resolved commit SHA before creating worktrees, and sanitizes every externally-sourced value (CVE ID, issue key, git SHA, email) before it is used in a URL, path, or git command. Signed-off-by: Mytreya Kasturi <mykastur@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mytreya-rh 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 |
openshift-ci
Bot
added
the
approved
WalkthroughThis change adds a reusable eight-step Go CVE triage library, a Helm Operator CLI driver, operational documentation, and repository ignore rules. The workflow integrates Jira, OSV, Go vulnerability data, Git worktrees, govulncheck, dependency analysis, and VTA callgraphs. ChangesGo CVE triage
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant HelmOperatorCLI
participant TriageCore
participant Jira
participant AdvisoryServices
participant GitAndAnalysis
Operator->>HelmOperatorCLI: Provide issue and analysis options
HelmOperatorCLI->>TriageCore: Start run_triage
TriageCore->>Jira: Retrieve issue and post evidence
TriageCore->>AdvisoryServices: Resolve CVE and Go vulnerability data
TriageCore->>GitAndAnalysis: Create worktree and run reachability checks
GitAndAnalysis-->>TriageCore: Return scan and callgraph results
TriageCore-->>HelmOperatorCLI: Return triage status
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mytreya-rh
changed the title
|
@mytreya-rh: This pull request references OAPE-906 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. |
openshift-ci-robot
added
the
jira/valid-reference
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
hack/cve-triage/README.md (1)
41-45: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the analysis tool versions.
@latestmakes Jira evidence non-reproducible. A newergovulncheckorcallgraphversion can change the result for the same commit SHA. Pin reviewed versions and define how updates are validated.Proposed documentation change
-go install golang.org/x/vuln/cmd/govulncheck@latest -go install golang.org/x/tools/cmd/callgraph@latest -go install golang.org/x/tools/cmd/digraph@latest +go install golang.org/x/vuln/cmd/govulncheck@<reviewed-version> +go install golang.org/x/tools/cmd/callgraph@<reviewed-version> +go install golang.org/x/tools/cmd/digraph@<reviewed-version>🤖 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 `@hack/cve-triage/README.md` around lines 41 - 45, Update the tool installation commands in the README to replace every `@latest` tag with explicitly reviewed, pinned versions for govulncheck, callgraph, and digraph. Document the validation process required before updating those versions so analysis results remain reproducible for the same commit SHA.hack/cve-triage/cve_triage_core.py (6)
719-725: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the loop variable
l.Ruff reports E741 on both lines.
lis easily confused with1andI. Uselabelorlbl.♻️ Proposed refactor
- labels = [l.lower() for l in (fields.get("labels") or [])] + labels = [lbl.lower() for lbl in (fields.get("labels") or [])] @@ - if any(l.startswith("golang") or l.startswith("go-") for l in labels): + if any(lbl.startswith("golang") or lbl.startswith("go-") for lbl in labels):🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 719 - 725, Rename the list-comprehension variable l to label (or lbl) in the labels assignment and update the startswith check to use the renamed variable, eliminating Ruff E741 without changing behavior.Source: Linters/SAST tools
229-234: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose the file handle and handle more error cases.
open(path).read()leaves the file object to the garbage collector. Use a context manager. A missing home-directory file can also raiseIsADirectoryErrororPermissionError, which currently propagates as an unhandled traceback instead of falling through to the next credential source.♻️ Proposed refactor
def _read_file_stripped(path: str): """Return the contents of path stripped of whitespace, or None if missing.""" try: - return open(path).read().strip() - except FileNotFoundError: + with open(path) as f: + return f.read().strip() + except OSError: return None🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 229 - 234, Update _read_file_stripped to open the file with a context manager and catch IsADirectoryError and PermissionError alongside FileNotFoundError, returning None for all these unavailable-file cases so credential lookup can continue.
348-356: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReport Jira HTTP status codes distinctly in
jira_get.
urllib.error.HTTPErrorsubclassesurllib.error.URLError. A 401 or 404 response therefore reaches theURLErrorhandler and is reported as a network error with a VPN hint.jira_postalready separates the two cases. Mirror that here so credential and permission failures are identifiable.♻️ Proposed refactor
try: with urllib.request.urlopen(req, timeout=30) as r: return json.loads(r.read()) + except urllib.error.HTTPError as e: + raise JiraError(f"GET {path} → {e.code}: {e.read().decode()}") from e except urllib.error.URLError as e: raise JiraError(_net_error_hint(e)) from e🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 348 - 356, Update jira_get to catch urllib.error.HTTPError before the broader URLError handler and raise the same distinct Jira HTTP-status error used by jira_post, while retaining _net_error_hint for genuine network failures.
943-946: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
_safe_output_pathcall here cannot fail.
base_dirisos.path.dirname(out_file)and the single part isos.path.basename(out_file). The candidate always resolves under the base, so the traversal check is a no-op. The real check already happens inrun_triageat line 2101. Remove the call to avoid suggesting a guarantee that is not provided. The same pattern appears at line 1382.🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 943 - 946, Remove the redundant _safe_output_path call in the output-writing flow before open, since dirname/basename of out_file cannot trigger traversal validation and run_triage already performs the real check. Apply the same removal to the matching output-writing pattern near the other occurrence, preserving directory creation and file writing.
632-688: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
awareis a dead signal.
awareisgo_id is not None. At the only call site, line 1908,go_idis always a non-empty string, because line 1865 assignsgo_id = cve_idin the fallback path.awareis therefore alwaysTrue, and the caller discards it (Ruff RUF059 at line 1908). Either removeawarefrom the return tuple or base it on a value that can actually be false, such as whether the local DB contains theGO-*entry.🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 632 - 688, Remove the dead aware value from check_govulncheck_awareness and its return tuple, then update the call site to unpack only the remaining values and avoid the discarded-variable warning. Preserve the existing govulncheck version, database timestamp, and warning behavior.Source: Linters/SAST tools
1026-1046: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid re-sorting the dependency set on every lookup.
pick_sanity_exampleandcheck_module_in_binary_depsboth callsorted(deps).depsholds the full transitive import set, which is typically thousands of entries.check_module_in_binary_depsis called once per affected package inside the loops at lines 1993 and 2188, so the sort repeats. Sort once and pass the sorted list, or accept a pre-sorted sequence.🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 1026 - 1046, Update pick_sanity_example and check_module_in_binary_deps to accept or reuse a pre-sorted dependency sequence instead of calling sorted(deps) internally. Create the sorted dependency list once before the affected lookup loops, including the callers around lines 1993 and 2188, and pass it through while preserving the existing matching behavior.hack/cve-triage/triage_helm_operator_cve.py (1)
135-142: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
--branchis silently ignored.The flag accepts a value, and
mainnever readsargs.branch. A user who passes--branch release-4.20receives analysis of the branches derived from the JiraaffectedVersioninstead, with no message. Emit a deprecation warning when the flag is supplied, or useargparse.SUPPRESSand reject the value.♻️ Proposed refactor
analysis.add_argument( "--branch", metavar="BRANCH", help=( "Deprecated/no-op: release branch is always derived from the Jira " "affectedVersion. Kept for CLI compatibility." ), )Then in
main, after parsing:if args.branch: print(f"⚠️ --branch is a no-op and is ignored: {args.branch!r}") print(" The release branch is derived from the Jira affectedVersion.")🤖 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 `@hack/cve-triage/triage_helm_operator_cve.py` around lines 135 - 142, Update main’s parsed-arguments handling for the deprecated --branch option so supplying a value emits a clear warning that the flag is ignored and the release branch comes from Jira affectedVersion; preserve the existing derived-branch analysis behavior.
🤖 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 `@hack/cve-triage/cve_triage_core.py`:
- Around line 2101-2114: The cached artifact filenames are keyed only by release
version, allowing results from an older commit to be reused. In
hack/cve-triage/cve_triage_core.py lines 2101-2114, update the govulncheck
artifact path around _safe_output_path to include commit_sha in addition to
version_tag; likewise update lines 2146-2160 for the callgraph artifact path.
Ensure both cache reads and writes use the commit-specific filenames.
- Around line 2080-2098: Restructure the no_vuln_db/module-presence branch so
its per-version processing inside the affected-version loop records evidence in
version_results instead of closing and returning immediately. Move the Jira
closure, success reporting, and final return to after the loop, matching the
symbol-level path, so all versions are checked before closing the ticket.
- Around line 1794-1802: Normalize nullable Jira custom-field values to an empty
string before calling .strip() in the fields extraction block, specifically for
cve_id and ps_component. Preserve the existing “No CVE ID found in issue”
handling and downstream allowlist behavior when either field is unset.
- Line 1409: Update the db_modified assignment in the relevant parsing flow to
handle an explicit None db_last_modified value before slicing, matching the
existing fallback pattern in build_jira_comment. Preserve the ten-character date
truncation for valid values and use "unknown" when the key is missing or None.
- Around line 557-584: Sanitize cve_id immediately after it is read in
run_triage, before any fast-path lookup, by reusing _sanitize_cve_id. Also
validate affected_version at that trust boundary against an explicit allow-list
pattern permitting only the expected version format, and reject or skip invalid
values before cve_triage_core builds JQL. Ensure every value reaching the JQL
construction around builder_labels and version_filters has passed these checks.
- Around line 811-819: Handle a None result from _git_rev in the existing
worktree validation block before slicing actual_sha. Treat an invalid or
unresolvable worktree as needing removal and recreation, while preserving the
reuse path when actual_sha matches commit_sha and only formatting the SHA prefix
when a value exists.
- Around line 1905-1919: Update the CVE triage flow around
check_govulncheck_awareness and warn so a stale govulncheck database aborts
processing before the auto-close path, unless the operator explicitly provides
the existing override mechanism. Preserve the current warning output, and ensure
the override is checked before govulncheck_found can lead to the “Not a Bug”
closure.
- Around line 1141-1161: Update _fmt_path to parse each digraph path as
individual space-separated nodes before constructing edge_map, adding an edge
between each consecutive pair. Preserve the existing chain formatting and
hop-count logic so multi-edge paths report the complete node chain.
In `@hack/cve-triage/README.md`:
- Around line 84-86: Add a language identifier such as text or console to the
fenced code blocks in the README, including the CLI example and the workflow
block around “[1] Fetch Jira” and lines 133–150, so all blocks satisfy
markdownlint MD040.
- Around line 113-114: Update the --dry-run entry in the README to describe the
mode as read-only and preventing Jira mutations, while retaining that it prints
the full analysis and permits re-analysis of closed tickets. Remove the claim
that it makes no Jira API calls, since the workflow still reads Jira data and
requires access.
- Around line 174-175: Update the README guidance for the callgraph step and
govulncheck outcome: do not state or imply that a missing govulncheck binary
means the binary is clean. Document that command-not-found and non-zero scan
failures must be handled as failures before accepting a run_govulncheck result,
even when config and SBOM checks pass.
---
Nitpick comments:
In `@hack/cve-triage/cve_triage_core.py`:
- Around line 719-725: Rename the list-comprehension variable l to label (or
lbl) in the labels assignment and update the startswith check to use the renamed
variable, eliminating Ruff E741 without changing behavior.
- Around line 229-234: Update _read_file_stripped to open the file with a
context manager and catch IsADirectoryError and PermissionError alongside
FileNotFoundError, returning None for all these unavailable-file cases so
credential lookup can continue.
- Around line 348-356: Update jira_get to catch urllib.error.HTTPError before
the broader URLError handler and raise the same distinct Jira HTTP-status error
used by jira_post, while retaining _net_error_hint for genuine network failures.
- Around line 943-946: Remove the redundant _safe_output_path call in the
output-writing flow before open, since dirname/basename of out_file cannot
trigger traversal validation and run_triage already performs the real check.
Apply the same removal to the matching output-writing pattern near the other
occurrence, preserving directory creation and file writing.
- Around line 632-688: Remove the dead aware value from
check_govulncheck_awareness and its return tuple, then update the call site to
unpack only the remaining values and avoid the discarded-variable warning.
Preserve the existing govulncheck version, database timestamp, and warning
behavior.
- Around line 1026-1046: Update pick_sanity_example and
check_module_in_binary_deps to accept or reuse a pre-sorted dependency sequence
instead of calling sorted(deps) internally. Create the sorted dependency list
once before the affected lookup loops, including the callers around lines 1993
and 2188, and pass it through while preserving the existing matching behavior.
In `@hack/cve-triage/README.md`:
- Around line 41-45: Update the tool installation commands in the README to
replace every `@latest` tag with explicitly reviewed, pinned versions for
govulncheck, callgraph, and digraph. Document the validation process required
before updating those versions so analysis results remain reproducible for the
same commit SHA.
In `@hack/cve-triage/triage_helm_operator_cve.py`:
- Around line 135-142: Update main’s parsed-arguments handling for the
deprecated --branch option so supplying a value emits a clear warning that the
flag is ignored and the release branch comes from Jira affectedVersion; preserve
the existing derived-branch analysis behavior.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 37e3ab76-b75e-4dc8-812a-d1b852186cf4
📒 Files selected for processing (4)
.gitignorehack/cve-triage/README.mdhack/cve-triage/cve_triage_core.pyhack/cve-triage/triage_helm_operator_cve.py
| version_base = re.sub(r"\.z$", "", affected_version) | ||
| version_z = version_base + ".z" | ||
| version_filters = list(dict.fromkeys([affected_version, version_z, version_base])) | ||
|
|
||
| builder_labels = [ | ||
| "pscomponent:openshift4/openshift-golang-builder-container", | ||
| "pscomponent:openshift-golang-builder-container", | ||
| ] | ||
| fields = ["summary", "status", "versions", "customfield_10669"] | ||
|
|
||
| for builder_label in builder_labels: | ||
| for version_filter in version_filters: | ||
| jql = ( | ||
| f'project = OCPBUGS AND labels = "{cve_id}" ' | ||
| f'AND labels = "{builder_label}" ' | ||
| f'AND affectedVersion = "{version_filter}"' | ||
| ) | ||
| results = jira_search(jql, fields, creds) | ||
| if results: | ||
| return results[0] | ||
|
|
||
| jql_any = ( | ||
| f'project = OCPBUGS AND labels = "{cve_id}" ' | ||
| f'AND labels = "{builder_label}"' | ||
| ) | ||
| results = jira_search(jql_any, fields, creds) | ||
| if results: | ||
| return results[0] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate cve_id and affected_version before building JQL.
Both values are interpolated into quoted JQL string literals. Both originate from Jira custom fields and are only passed through os.path.basename at lines 1801-1802, which does not remove " or JQL operators. _sanitize_cve_id is applied later, at line 1924, after this fast-path already ran. A ticket field containing a double quote changes the query semantics, so a wrong openshift-golang-builder-container ticket can be matched and the source ticket can be closed as a duplicate of it.
Apply _sanitize_cve_id(cve_id) immediately after reading the field at line 1794, and validate each affected version against an explicit pattern.
🛡️ Proposed fix
+_VERSION_RE = re.compile(r"^\d+\.\d+(\.\d+)?(\.z)?$")
+
+
+def _sanitize_version(version: str) -> str:
+ """Raise ValueError unless version looks like 4.21 / 4.21.z / 4.21.1."""
+ if not _VERSION_RE.fullmatch(version):
+ raise ValueError(f"Unexpected affected version format: {version!r}")
+ return version
+
+
def find_golang_builder_ticket(cve_id: str, affected_version: str, creds: JiraCreds):
"""
Search OCPBUGS for an openshift-golang-builder-container ticket for this
CVE and OCP release. Returns the Jira issue dict or None.
"""
+ cve_id = _sanitize_cve_id(cve_id)
+ affected_version = _sanitize_version(affected_version)
version_base = re.sub(r"\.z$", "", affected_version)Then validate at the source in run_triage:
- cve_id = os.path.basename(cve_id) if cve_id else cve_id
- affects = [os.path.basename(v) for v in affects]
+ cve_id = _sanitize_cve_id(cve_id) if cve_id else cve_id
+ affects = [_sanitize_version(v) for v in affects]As per path instructions: "Validate at trust boundaries with allow-lists, not deny-lists".
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 557 - 584, Sanitize cve_id
immediately after it is read in run_triage, before any fast-path lookup, by
reusing _sanitize_cve_id. Also validate affected_version at that trust boundary
against an explicit allow-list pattern permitting only the expected version
format, and reject or skip invalid values before cve_triage_core builds JQL.
Ensure every value reaching the JQL construction around builder_labels and
version_filters has passed these checks.
Source: Path instructions
| if os.path.isdir(worktree_path): | ||
| actual_sha = _git_rev(worktree_path, "HEAD") | ||
| if actual_sha == commit_sha: | ||
| return False | ||
| print( | ||
| f" ⚠️ Worktree HEAD {actual_sha[:12]} ≠ " | ||
| f"{commit_sha[:12]} — recreating." | ||
| ) | ||
| remove_worktree(repo_root, worktree_path) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
actual_sha can be None, which crashes the message formatting.
_git_rev returns None when git rev-parse HEAD exits non-zero. That happens when worktree_path exists but is not a valid worktree, for example a leftover empty directory or a directory whose .git file points at a pruned registration. The comparison at line 813 then fails, and line 816 evaluates actual_sha[:12], which raises TypeError: 'NoneType' object is not subscriptable. run_triage catches only RuntimeError and ValueError at lines 1951-1956, so the whole run aborts with a traceback.
🐛 Proposed fix
if os.path.isdir(worktree_path):
actual_sha = _git_rev(worktree_path, "HEAD")
if actual_sha == commit_sha:
return False
print(
- f" ⚠️ Worktree HEAD {actual_sha[:12]} ≠ "
+ f" ⚠️ Worktree HEAD {actual_sha[:12] if actual_sha else 'unresolvable'} ≠ "
f"{commit_sha[:12]} — recreating."
)
remove_worktree(repo_root, worktree_path)
+ if os.path.isdir(worktree_path):
+ shutil.rmtree(worktree_path, ignore_errors=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if os.path.isdir(worktree_path): | |
| actual_sha = _git_rev(worktree_path, "HEAD") | |
| if actual_sha == commit_sha: | |
| return False | |
| print( | |
| f" ⚠️ Worktree HEAD {actual_sha[:12]} ≠ " | |
| f"{commit_sha[:12]} — recreating." | |
| ) | |
| remove_worktree(repo_root, worktree_path) | |
| if os.path.isdir(worktree_path): | |
| actual_sha = _git_rev(worktree_path, "HEAD") | |
| if actual_sha == commit_sha: | |
| return False | |
| print( | |
| f" ⚠️ Worktree HEAD {actual_sha[:12] if actual_sha else 'unresolvable'} ≠ " | |
| f"{commit_sha[:12]} — recreating." | |
| ) | |
| remove_worktree(repo_root, worktree_path) | |
| if os.path.isdir(worktree_path): | |
| shutil.rmtree(worktree_path, ignore_errors=True) |
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 811 - 819, Handle a None
result from _git_rev in the existing worktree validation block before slicing
actual_sha. Treat an invalid or unresolvable worktree as needing removal and
recreation, while preserving the reuse path when actual_sha matches commit_sha
and only formatting the SHA prefix when a value exists.
| def _fmt_path(output, label, node): | ||
| edge_map = {} | ||
| for line in output.splitlines(): | ||
| parts = line.strip().split(None, 1) | ||
| if len(parts) == 2: | ||
| edge_map[parts[0]] = parts[1] | ||
| chain = [] | ||
| cur = main_func | ||
| seen = set() | ||
| while cur and cur not in seen: | ||
| chain.append(cur) | ||
| seen.add(cur) | ||
| cur = edge_map.get(cur) | ||
| hops = len(chain) - 1 | ||
| chain_lines = [f" {chain[0]}"] | ||
| for step in chain[1:]: | ||
| chain_lines.append(f" → {step}") | ||
| return [ | ||
| f"✓ [{label}]: {node} ({hops} hop{'s' if hops != 1 else ''})", | ||
| _cmd_str(node), | ||
| ] + chain_lines |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
golang.org/x/tools/cmd/digraph somepath output format one node per line
💡 Result:
The digraph command from golang.org/x/tools/cmd/digraph does not have a specific "one node per line" flag for the somepath query [1][2]. By default, digraph somepath outputs the nodes found on a path between two nodes separated by spaces [3][4][1]. If you need the output formatted as one node per line, you can achieve this by piping the output of the digraph command into a tool like tr to replace spaces with newlines [3]. Example usage: digraph somepath startNode endNode < graph.txt | tr ' ' '\n' The digraph tool is designed to integrate into standard UNIX pipelines [3]. While its own output format for paths is space-separated [3][4], other commands like nodes and sccs natively output one node per line [5][1][6], and the tool consistently outputs one edge per line for commands that return edges (like transpose) [5][7][6].
Citations:
- 1: https://android.googlesource.com/platform/tools/external/go/src/golang.org/x/tools/+/780b9c6dfe63042bdcc0aa28b0d51c81210b9529/cmd/digraph/digraph.go
- 2: https://deepwiki.com/golang/tools/8.1-developer-utilities
- 3: https://pkg.go.dev/golang.org/x/tools/cmd/digraph
- 4: https://go.googlesource.com/tools/+/refs/tags/gopls/v0.4.0/cmd/digraph/digraph.go
- 5: https://go.googlesource.com/tools/+/908cf394a88b2cfae87691a66da2dea8453e61cd/cmd/digraph/digraph.go
- 6: https://go.googlesource.com/tools/+/8918ecb06a5982454f7547d4e945f08f971067f4/cmd/digraph/digraph.go
- 7: https://go.googlesource.com/tools/+/7f6d66f278ca65549cc64456cd18d7489ad7d1f6/cmd/digraph/digraph.go
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant definitions and call sites ---'
rg -n -C 8 'digraph|_fmt_path|somepath|sanity_check_callgraph' hack/cve-triage/cve_triage_core.py
printf '%s\n' '--- digraph source references in repository ---'
rg -n -C 3 'somepath|cmd/digraph' .Repository: openshift/ocp-release-operator-sdk
Length of output: 21647
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
p = Path("hack/cve-triage/cve_triage_core.py")
text = p.read_text()
start = text.index(" def _fmt_path(")
end = text.index("\n label_names =", start)
src = text[start:end]
print("--- _fmt_path implementation ---")
print(src)
def emulate(output, main_func):
edge_map = {}
for line in output.splitlines():
parts = line.strip().split(None, 1)
if len(parts) == 2:
edge_map[parts[0]] = parts[1]
chain = []
cur = main_func
seen = set()
while cur and cur not in seen:
chain.append(cur)
seen.add(cur)
cur = edge_map.get(cur)
return chain, len(chain) - 1, edge_map
for output in [
"main.func callee.func leaf.func",
"main.func\ncallee.func\nleaf.func",
'"main.func" "callee.func"\n"callee.func" "leaf.func"',
]:
chain, hops, edge_map = emulate(output, "main.func")
print(f"--- output={output!r} ---")
print(f"edge_map={edge_map!r}")
print(f"chain={chain!r}, hops={hops}")
PYRepository: openshift/ocp-release-operator-sdk
Length of output: 1337
🌐 Web query:
official golang.org/x/tools/cmd/digraph source somepath output fmt.Printf
💡 Result:
The digraph tool, located in the golang.org/x/tools/cmd/digraph package, is a command-line utility used to perform queries over unlabelled directed graphs [1][2]. Regarding your query about the somepath command and how it handles output, the source code for the tool uses fmt.Fprintf to report errors to standard error (os.Stderr) [3][4][5] and typically writes the results of its graph operations (including somepath) to standard output (os.Stdout) [1][5]. For the somepath command specifically, the implementation executes a graph traversal to find a path between two nodes [1][3]. When successful, the tool emits the list of nodes on that arbitrary path to standard output [1]. If an error occurs during processing—such as if the input graph is invalid or the command is used with incorrect arguments—the tool reports the error to os.Stderr and exits with a non-zero status code [3][4][5]. The general syntax for the tool is: your-application | digraph [command] [args] Example usage for somepath: cat graph.txt | digraph somepath nodeA nodeB In the source code, the handling of the somepath command is structured within the main processing loop where it validates that exactly two arguments are provided [4][5]. If the arguments are invalid, it returns an error that is subsequently formatted and printed to os.Stderr [3][4]. Otherwise, it performs the path search and writes the result to the configured output stream [5].
Citations:
- 1: https://pkg.go.dev/golang.org/x/tools/cmd/digraph
- 2: https://github.com/golang/tools/
- 3: https://go.googlesource.com/tools/+/refs/tags/gopls/v0.4.0/cmd/digraph/digraph.go
- 4: https://android.googlesource.com/platform/tools/external/go/src/golang.org/x/tools/+/780b9c6dfe63042bdcc0aa28b0d51c81210b9529/cmd/digraph/digraph.go
- 5: https://go.googlesource.com/tools/+/7f6d66f278ca65549cc64456cd18d7489ad7d1f6/cmd/digraph/digraph.go
Parse digraph somepath paths before formatting.
The command emits nodes as a space-separated path. For paths with more than one edge, the current parser treats all remaining nodes as one node and reports an incorrect hop count and chain.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 1158-1161: Consider iterable unpacking instead of concatenation
Replace with iterable unpacking
(RUF005)
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 1141 - 1161, Update
_fmt_path to parse each digraph path as individual space-separated nodes before
constructing edge_map, adding an edge between each consecutive pair. Preserve
the existing chain formatting and hop-count logic so multi-edge paths report the
complete node chain.
| fixed = (go_data or {}).get("fixed", "N/A") | ||
| go_id = (go_data or {}).get("go_id", "N/A") | ||
| pkg_list = "\n".join(f"- `{p}`" for p in (go_data or {}).get("packages", [])) | ||
| db_modified = (db_info or {}).get("db_last_modified", "unknown")[:10] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
db_last_modified can be None, so the slice raises TypeError.
_parse_govulncheck_output always sets the db_last_modified key at line 897, using cfg.get("db_last_modified"). If the govulncheck config block omits that field, the key exists with value None. dict.get then returns None, not the "unknown" default, and None[:10] raises TypeError. build_jira_comment already handles this correctly at line 1545. Apply the same pattern here.
🐛 Proposed fix
- db_modified = (db_info or {}).get("db_last_modified", "unknown")[:10]
- scanner_ver = (db_info or {}).get("scanner_version", "unknown")
- go_ver = (db_info or {}).get("go_version", "unknown")
+ db_modified = ((db_info or {}).get("db_last_modified") or "unknown")[:10]
+ scanner_ver = (db_info or {}).get("scanner_version") or "unknown"
+ go_ver = (db_info or {}).get("go_version") or "unknown"🤖 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 `@hack/cve-triage/cve_triage_core.py` at line 1409, Update the db_modified
assignment in the relevant parsing flow to handle an explicit None
db_last_modified value before slicing, matching the existing fallback pattern in
build_jira_comment. Preserve the ten-character date truncation for valid values
and use "unknown" when the key is missing or None.
| cve_id = fields.get("customfield_10667", "").strip() | ||
| ps_component = fields.get("customfield_10669", "").strip() | ||
| status = fields["status"]["name"] | ||
| affects = [v["name"] for v in fields.get("versions", [])] | ||
| tech_field = fields.get("customfield_10632", "") | ||
| # Sanitize Jira-sourced values used as file-system path components. | ||
| # os.path.basename is recognized by Snyk as a path-traversal sanitizer. | ||
| cve_id = os.path.basename(cve_id) if cve_id else cve_id | ||
| affects = [os.path.basename(v) for v in affects] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unset Jira custom fields return null, which breaks .strip().
Jira returns null for an unset custom field, so fields.get("customfield_10667", "") returns None and .strip() raises AttributeError. The intended handling at line 1811 ("No CVE ID found in issue") is then never reached, and the same applies to ps_component at line 1795 and the allowlist check at line 1815. Normalize the value before calling .strip().
🐛 Proposed fix
- cve_id = fields.get("customfield_10667", "").strip()
- ps_component = fields.get("customfield_10669", "").strip()
+ cve_id = (fields.get("customfield_10667") or "").strip()
+ ps_component = (fields.get("customfield_10669") or "").strip()
status = fields["status"]["name"]
affects = [v["name"] for v in fields.get("versions", [])]
- tech_field = fields.get("customfield_10632", "")
+ tech_field = fields.get("customfield_10632") or ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cve_id = fields.get("customfield_10667", "").strip() | |
| ps_component = fields.get("customfield_10669", "").strip() | |
| status = fields["status"]["name"] | |
| affects = [v["name"] for v in fields.get("versions", [])] | |
| tech_field = fields.get("customfield_10632", "") | |
| # Sanitize Jira-sourced values used as file-system path components. | |
| # os.path.basename is recognized by Snyk as a path-traversal sanitizer. | |
| cve_id = os.path.basename(cve_id) if cve_id else cve_id | |
| affects = [os.path.basename(v) for v in affects] | |
| cve_id = (fields.get("customfield_10667") or "").strip() | |
| ps_component = (fields.get("customfield_10669") or "").strip() | |
| status = fields["status"]["name"] | |
| affects = [v["name"] for v in fields.get("versions", [])] | |
| tech_field = fields.get("customfield_10632") or "" | |
| # Sanitize Jira-sourced values used as file-system path components. | |
| # os.path.basename is recognized by Snyk as a path-traversal sanitizer. | |
| cve_id = os.path.basename(cve_id) if cve_id else cve_id | |
| affects = [os.path.basename(v) for v in affects] |
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 1794 - 1802, Normalize
nullable Jira custom-field values to an empty string before calling .strip() in
the fields extraction block, specifically for cve_id and ps_component. Preserve
the existing “No CVE ID found in issue” handling and downstream allowlist
behavior when either field is unset.
| print(f"\n[8] Closing {issue_key} as Not a Bug...") | ||
| if dry_run: | ||
| print("\n--- [DRY-RUN] Jira comment that would be posted ---") | ||
| print(jira_comment) | ||
| print("---") | ||
| else: | ||
| try: | ||
| close_as_not_affected(issue_key, jira_comment, cfg, creds) | ||
| except JiraError as e: | ||
| print(f"\n❌ Jira operation failed: {e}") | ||
| return 1 | ||
|
|
||
| print(f"\n{'='*60}") | ||
| print(f"✅ Done! {issue_key} → Closed as Not a Bug (module-presence check)") | ||
| if dry_run: | ||
| print(" (dry-run: no changes made to Jira)") | ||
| print(f" Report: {report_file}") | ||
| print(f"{'='*60}\n") | ||
| return 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The module-presence path closes the ticket after checking only one version.
This block sits inside the for affected_version in versions_to_analyze loop and ends with return 0. When a ticket lists more than one affected version, the code analyzes the first version, closes the ticket as "Not a Bug", and returns. The remaining versions are never checked, yet the closure applies to the whole ticket.
The symbol-level path below handles this correctly. It appends to version_results per version and closes once, after the loop, at lines 2245-2276. Restructure the no_vuln_db branch the same way: accumulate per-version evidence, then close after the loop completes.
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 2080 - 2098, Restructure the
no_vuln_db/module-presence branch so its per-version processing inside the
affected-version loop records evidence in version_results instead of closing and
returning immediately. Move the Jira closure, success reporting, and final
return to after the loop, matching the symbol-level path, so all versions are
checked before closing the ticket.
| govulncheck_out = _safe_output_path(cve_out_dir, f"govulncheck-{version_tag}.json") | ||
| print(f"\n[5] Running govulncheck (symbol-level) on {path_branch}...") | ||
| print(f" Output: {govulncheck_out}") | ||
|
|
||
| if os.path.exists(govulncheck_out) and not force: | ||
| print(f" ℹ️ Using cached govulncheck output (--force to re-run)") | ||
| with open(govulncheck_out) as f: | ||
| raw_cached = f.read() | ||
| govulncheck_found, db_info, _, module_versions, has_config, has_sbom = \ | ||
| _parse_govulncheck_output(raw_cached, go_id) | ||
| if not has_config or not has_sbom: | ||
| print(f"\n❌ Cached govulncheck output is incomplete (missing config or SBOM).") | ||
| print(f" Re-run with --force to get a fresh scan.") | ||
| return 1 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Cache artifact paths are keyed by release version, not by the analyzed commit. Both cached artifacts use version_tag, which derives from the Jira affectedVersion. The worktree, however, is pinned to commit_sha resolved at line 1948. After the release branch advances, a run without --force reuses artifacts produced from older source, and the workflow closes the ticket as "Not a Bug" based on that stale evidence.
hack/cve-triage/cve_triage_core.py#L2101-L2114: includecommit_shain thegovulncheck-{version_tag}.jsonfilename.hack/cve-triage/cve_triage_core.py#L2146-L2160: includecommit_shain thecallgraph-{version_tag}.txtfilename.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 2106-2106: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(govulncheck_out)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.16.1)
[error] 2106-2106: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 2106-2106: String contains ambiguous ℹ (INFORMATION SOURCE). Did you mean i (LATIN SMALL LETTER I)?
(RUF001)
[error] 2112-2112: f-string without any placeholders
Remove extraneous f prefix
(F541)
[error] 2113-2113: f-string without any placeholders
Remove extraneous f prefix
(F541)
📍 Affects 1 file
hack/cve-triage/cve_triage_core.py#L2101-L2114(this comment)hack/cve-triage/cve_triage_core.py#L2146-L2160
🤖 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 `@hack/cve-triage/cve_triage_core.py` around lines 2101 - 2114, The cached
artifact filenames are keyed only by release version, allowing results from an
older commit to be reused. In hack/cve-triage/cve_triage_core.py lines
2101-2114, update the govulncheck artifact path around _safe_output_path to
include commit_sha in addition to version_tag; likewise update lines 2146-2160
for the callgraph artifact path. Ensure both cache reads and writes use the
commit-specific filenames.
| ``` | ||
| python3 hack/cve-triage/triage_helm_operator_cve.py <issue> [options] | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add languages to the fenced code blocks.
markdownlint reports MD040 for the CLI and workflow blocks. Mark the blocks as text or console.
Proposed fix
-```
+```text
python3 hack/cve-triage/triage_helm_operator_cve.py <issue> [options]- +text
[1] Fetch Jira
</details>
Also applies to: 133-150
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.23.2)</summary>
[warning] 84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @hack/cve-triage/README.md around lines 84 - 86, Add a language identifier
such as text or console to the fenced code blocks in the README, including the
CLI example and the workflow block around “[1] Fetch Jira” and lines 133–150, so
all blocks satisfy markdownlint MD040.
</details>
<!-- fingerprinting:phantom:triton:caracal -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:131043c25d5b9198b92807d9 -->
_Source: Linters/SAST tools_
<!-- This is an auto-generated comment by CodeRabbit -->
| | `--dry-run` | Print full analysis without making any Jira API calls. Skips the already-closed early exit so you can re-analyse a closed ticket. | | ||
| | `--branch BRANCH` | Deprecated/no-op — kept for CLI compatibility. The release branch is always derived from Jira `affectedVersion`; see "Security hardening" below. | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe --dry-run as read-only, not offline.
The workflow still fetches Jira data to obtain the CVE, affected version, component, and status. The CLI context also describes dry-run as avoiding Jira mutations, not all Jira API calls. Update this text to avoid implying that credentials or network access are unnecessary.
Proposed wording
-| `--dry-run` | Print full analysis without making any Jira API calls. Skips the already-closed early exit so you can re-analyse a closed ticket. |
+| `--dry-run` | Print full analysis without posting or closing Jira issues. Skips the already-closed early exit so you can re-analyse a closed ticket. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `--dry-run` | Print full analysis without making any Jira API calls. Skips the already-closed early exit so you can re-analyse a closed ticket. | | |
| | `--branch BRANCH` | Deprecated/no-op — kept for CLI compatibility. The release branch is always derived from Jira `affectedVersion`; see "Security hardening" below. | | |
| | `--dry-run` | Print full analysis without posting or closing Jira issues. Skips the already-closed early exit so you can re-analyse a closed ticket. | | |
| | `--branch BRANCH` | Deprecated/no-op — kept for CLI compatibility. The release branch is always derived from Jira `affectedVersion`; see "Security hardening" below. | |
🤖 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 `@hack/cve-triage/README.md` around lines 113 - 114, Update the --dry-run entry
in the README to describe the mode as read-only and preventing Jira mutations,
while retaining that it prints the full analysis and permits re-analysis of
closed tickets. Remove the claim that it makes no Jira API calls, since the
workflow still reads Jira data and requires access.
| - The callgraph step takes 2–5 minutes. Use `--no-callgraph` when the | ||
| govulncheck result alone is sufficient (govulncheck not found → binary is clean). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'govulncheck|FileNotFoundError|CalledProcessError|not found|Not Affected|return 0' \
hack/cve-triage/cve_triage_core.pyRepository: openshift/ocp-release-operator-sdk
Length of output: 34125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README context ---'
cat -n hack/cve-triage/README.md | sed -n '150,190p'
printf '%s\n' '--- core definitions and call sites ---'
cat -n hack/cve-triage/cve_triage_core.py | sed -n '922,975p;1898,1930p;2098,2132p;2158,2170p;2240,2290p'
printf '%s\n' '--- related drivers, tests, and documentation ---'
rg -n -C 4 'no-callgraph|run_govulncheck|GOVULNCHECK|returncode|Not Affected|Not a Bug' hack/cve-triageRepository: openshift/ocp-release-operator-sdk
Length of output: 31030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("hack/cve-triage/cve_triage_core.py")
text = p.read_text()
for needle in ("def run_govulncheck", "if govulncheck_found:", "if not no_cg_this_ver:", "return 0"):
print(f"{needle}: {text.count(needle)} occurrence(s)")
PYRepository: openshift/ocp-release-operator-sdk
Length of output: 314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- driver entrypoint and exception handling ---'
cat -n hack/cve-triage/triage_helm_operator_cve.py | tail -n 70
printf '%s\n' '--- all invocations of run_triage and top-level handlers ---'
rg -n -C 8 'run_triage|except Exception|SystemExit|sys\.exit|return result' hack/cve-triage --glob '*.py'
printf '%s\n' '--- AST check of run_govulncheck subprocess handling ---'
python3 - <<'PY'
import ast
from pathlib import Path
source = Path("hack/cve-triage/cve_triage_core.py").read_text()
tree = ast.parse(source)
fn = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == "run_govulncheck")
runs = [n for n in ast.walk(fn) if isinstance(n, ast.Call)
and isinstance(n.func, ast.Attribute) and n.func.attr == "run"]
print("subprocess.run calls:", len(runs))
print("returncode reads:", sum(
isinstance(n, ast.Attribute) and n.attr == "returncode" for n in ast.walk(fn)
))
print("caught exception types:", [
ast.unparse(h.type) if h.type else "bare"
for n in ast.walk(fn) if isinstance(n, ast.Try)
for h in n.handlers
])
print("return tuple length:", next(
len(n.value.elts) for n in ast.walk(fn)
if isinstance(n, ast.Return) and isinstance(n.value, ast.Tuple)
))
PYRepository: openshift/ocp-release-operator-sdk
Length of output: 31763
🌐 Web query:
govulncheck exit status non-zero vulnerabilities JSON mode official documentation
💡 Result:
When running govulncheck, the exit status behavior depends on whether you use the standard text output or a machine-readable format like JSON [1][2]. Exit Status Behavior: - Standard (Text) Mode: Govulncheck exits with code 3 if vulnerabilities are detected and code 0 if none are found [3][4]. - JSON/SARIF/OpenVEX Mode: When using -json, -format sarif, or -format openvex, govulncheck always exits with code 0, regardless of whether vulnerabilities are detected [1][2]. Official Guidance: This behavior is intentional [5]. In JSON mode, an exit status of 0 indicates that the tool executed successfully and the JSON output is trustworthy [5]. It does not indicate the absence of vulnerabilities [5]. To determine if vulnerabilities were found when using JSON mode, you must parse the resulting JSON output to check for the presence of "finding" objects [5][6]. Working with JSON Output: Govulncheck uses a streaming JSON format, which means it emits a sequence of Message objects [7][8]. Because it emits findings as it discovers them, you should process the entire stream [7][6]. If a vulnerability is found, the JSON stream will contain a Finding object [7][8]. If no Finding objects are present in the stream, no vulnerabilities were detected [6].
Citations:
- 1: https://go.googlesource.com/vuln/+/ffdef74cc44d7eb71931d8d414c478b966812488/cmd/govulncheck/doc.go
- 2: https://go.googlesource.com/vuln.git/+/29462d73a2bf15636e29dc1e570ea0caa38dd5d5%5E%21/
- 3: https://go.googlesource.com/vuln/+/3ad0a6f964e9/cmd/govulncheck/doc.go
- 4: https://go.googlesource.com/vuln/+/v1.1.4/internal/scan/errors.go
- 5: x/vuln: json output always exits 0 golang/go#61704
- 6: x/vuln/cmd/govulncheck: output a final result in json mode golang/go#62340
- 7: https://pkg.go.dev/golang.org/x/vuln/internal/govulncheck
- 8: https://go.googlesource.com/vuln/+/v1.3.0/internal/govulncheck/govulncheck.go
Do not describe a missing govulncheck binary as a clean result.
A missing binary currently raises FileNotFoundError, so it does not produce a clean verdict. run_govulncheck also ignores non-zero exit statuses; a failed scan can pass the config and SBOM checks and return found=False. Handle command-not-found and scan failures before accepting that result.
🤖 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 `@hack/cve-triage/README.md` around lines 174 - 175, Update the README guidance
for the callgraph step and govulncheck outcome: do not state or imply that a
missing govulncheck binary means the binary is clean. Document that
command-not-found and non-zero scan failures must be handled as failures before
accepting a run_govulncheck result, even when config and SBOM checks pass.
|
@mytreya-rh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Description of the change
Adds
hack/cve-triage/, a reusable govulncheck + VTA callgraph CVE triageworkflow for Go binaries, plus a driver (
triage_helm_operator_cve.py)wired up for the
ose-helm-operator/ose-helm-rhel9-operatorPScomponents.
For a given OCPBUGS Jira ticket, the tool:
and the OSV / vuln.go.dev databases).
openshift-golang-builder-containerticket via duplicate-linking.
govulncheck's local DB is aware of the CVE.git worktreefor the affected release branch,pinned to a resolved commit SHA.
govulncheck -jsonscoped to./cmd/helm-operator/forsymbol-level reachability.
callgraph -algo vta) andqueries
digraph somepathfor every vulnerable symbol, validating thepipeline against known-reachable nodes first.
.work/compliance/analyze-cve/.evidence comment to Jira and closes the ticket as Not a Bug.
The core library (
cve_triage_core.py) is intentionally hardened forstatic analysis / SSRF / injection concerns, since it drives outbound
network requests and local git/file operations from Jira-sourced and
CLI-sourced input:
urlopen()call is checked against a host allowlist(
redhat.atlassian.net,api.osv.dev,vuln.go.dev) before therequest is issued.
JiraCredsobject from thecomponent config (
TriageConfig) used to build local file paths.validated before use.
(
resolve_remote_sha) rather than a raw branch name.directory (
_safe_output_path).run_triage); theactual Jira-mutating functions never take a
dry_runparameter.--dry-runruns the complete analysis (including for already-closedtickets) and prints what would be posted to Jira, without making any
API calls.
cve_triage_core.pyhas no product-specific logic, so it can be reusedfor other Go operator images by writing a small driver (see the
"Adapting for another component" section of the README).
Motivation for the change
CVE scanners flag any Go package present in a binary's dependency graph,
regardless of whether the vulnerable code is reachable. Triaging these
for the helm-operator image today is a manual, repetitive process of
running
govulncheck/callgraphby hand and writing up evidence forJira. This tool automates that analysis and evidence-gathering so a
human only needs to review the dry-run output before closing a ticket.
Checklist
hack/) — nochangelog fragment or docs website update needed.
Made with Cursor
Summary by CodeRabbit
New Features
Documentation
Chores