Skip to content

CVE fix: Dual ecosystem scan - #101

Draft
celdrake wants to merge 2 commits into
flightctl:mainfrom
celdrake:cve-fix/dual-ecosystem-scan
Draft

CVE fix: Dual ecosystem scan#101
celdrake wants to merge 2 commits into
flightctl:mainfrom
celdrake:cve-fix/dual-ecosystem-scan

Conversation

@celdrake

@celdrake celdrake commented Aug 10, 2026

Copy link
Copy Markdown

PR has been created off #76 and will be rebased off main once that is merged. - Changes are only in last commit a13ff32

While working on #76, I noticed that ultimately the scan module determines a single language based on the presence of certain files in the repository.

For projects such as flightctl-ui which combine more that one of those languages, this detection could lead to having false negatives.

The updated approach extracts the "primary" language for the project and CVE, writes it to context.md and is used later for scanning the CVE presence or absence.

Example from flightctl-ui:

Node JS CVE

## Source
- Input type: jira_ticket
- Jira ticket: EDM-4513 — CVE-2026-13676 rhem/flightctl-ui-ocp-rhel9: fast-uri: Security policy bypass due to improper Unicode hostname canonicalization [rhem-1.2]
- Link: https://redhat.atlassian.net/browse/EDM-4513

...
## Detected Ecosystem(s)
- Node.js: npm (package.json / package-lock.json) [primary]
- Go: go (proxy/go.mod) (secondary; backend proxy — not the CVE package)

Go CVE

## Source
- Input type: jira_ticket
- Jira ticket: EDM-3944 — CVE-2026-32281 redhat-user-workloads/flightctl-ui: Go crypto/x509: Denial of Service via inefficient certificate chain validation [rhem-1.0]
- Link: https://redhat.atlassian.net/browse/EDM-3944
- Direct CVE input: n/a
- Ticket status: Closed (Done) — VEX Justification set to "Vulnerable Code not Present" (2026-06-29)

...

## Detected Ecosystem(s)
- Go: go (proxy/go.mod) [primary] — nested module `github.com/flightctl/flightctl-ui`; stdlib CVE targets this Go proxy
- Node.js: npm (package.json + package-lock.json) — UI monorepo workspaces; not primary for this CVE

Workflows affected

  • The start workflow now detects nested Go modules and records primary and additional ecosystems in context.md.
  • The scan workflow now selects the CVE’s primary ecosystem through LANGUAGE.
  • The scan workflow passes repository context through environment variables.
  • Go scans now cover all eligible modules and aggregate per-module verdicts.
  • Scan results now include per-module results and support fixed-version comparisons.
  • VEX closure now requires review of each affected module.

Skills, commands, and guidelines

  • scan.py now validates supported languages and detects nested Go modules.
  • The scanner now handles tool-only Go modules, semantic-version comparisons, fixed-version verdicts, and verdict priorities.
  • Manifest checks can inspect all Go modules in a repository.
  • New unit tests cover language selection, Go module discovery, version handling, and verdict aggregation.
  • SKILL.md version changed from 0.3.1 to 0.5.0.
  • start.md and scan.md now define polyglot repository handling and primary ecosystem reporting.

Shared resources and conventions

  • No changes affect _shared/ resources.
  • The workflows introduce cross-workflow conventions for context.md, LANGUAGE, primary ecosystem selection, and repository context propagation.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The scanner now supports explicit ecosystem selection, recursive multi-module Go scanning, fixed-version comparison, per-module results, and aggregated verdicts. Workflow guidance and tests cover nested modules, polyglot repositories, ignored directories, and version handling.

Changes

Go repository scanning

Layer / File(s) Summary
Ecosystem selection and module discovery
cve-fix/scripts/scan.py, cve-fix/scripts/test_scan.py, cve-fix/skills/start.md
The scanner resolves supported ecosystems, detects nested Go modules, excludes specified directories, and records the primary ecosystem.
Module version and verdict evaluation
cve-fix/scripts/scan.py, cve-fix/scripts/test_scan.py, cve-fix/skills/scan.md
Go modules use resolved and fixed versions for verdicts. Tool-only scan failures and aggregate verdict priority are supported.
CLI, manifest, and report integration
cve-fix/scripts/scan.py, cve-fix/skills/scan.md, cve-fix/SKILL.md
The CLI passes ecosystem and fixed-version settings, scans repository-root Go modules, preserves other scan paths, and reports per-module results.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: workflow-structure, scripts

Suggested reviewers: amir-yogev-gh, adalton

🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Ai-Attribution ✅ Passed Both PR commits attribute AI use with the allowed Made-with: Cursor trailer, and neither uses Co-Authored-By for AI.
No-Absolute-Paths-In-Skills ✅ Passed The PR's markdown additions contain no absolute filesystem paths; slash-prefixed text is limited to workflow commands or relative-path syntax.
Skill-Md-Under-30-Lines ✅ Passed The only changed SKILL.md is cve-fix/SKILL.md, and it contains 25 lines including frontmatter, below the 30-line limit.
Command-Colon-Notation ✅ Passed All 75 files under commands/ have YAML frontmatter names matching their workflow directory and the exact workflow:phase pattern; no invalid names were found.
No-Orphaned-References ✅ Passed Changed Markdown references resolve to existing workflow files; scan.py, controller.md, and guidelines.md exist. All 8 commands are activated in SKILL.md, and all 8 phase skills are listed by...
No-Content-Duplication ✅ Passed cve-fix/SKILL.md changed only its version; comparison found no shared multi-line instruction blocks or exact paragraphs with guidelines.md or skills/controller.md.
Step-Sequencing ✅ Passed Changed skills/scan.md has Steps 1–5 and skills/start.md has Steps 1–6; both are sequential, unique, and under 10, with no Step-letter substeps.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: CVE scanning now supports repositories with multiple ecosystems.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Comment thread cve-fix/scripts/scan.py
@@ -52,20 +61,41 @@


def detect_language(work_dir: Path) -> str:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing detection feature (both before and after PR76 can misidentify a CVE as present or not given it may classify the CVE as affecting the wrong module).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🤖 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 `@cve-fix/scripts/scan.py`:
- Around line 273-291: Update scan_go_tool_module so its “Module” output uses
the repository-relative module directory rather than the absolute mod_dir path.
Reuse the relative directory already computed by scan_go_repository and pass it
into the function, or add an equivalent display-name parameter while preserving
the existing scan result behavior.
- Around line 424-434: The per-module scan loop can multiply the total timeout
by the number of modules. Update the scanning flow around scan_go and
scan_go_tool_module to enforce a bounded overall budget, such as dividing the
available SCAN_TIMEOUT across discovered modules while preserving retry
behavior, or document the resulting worst-case duration in the scan guidance;
ensure the documented behavior matches the implemented time budget.
- Around line 754-756: Update the base-image override in determine_verdict to
require a failed scan, using the existing scanner-failure condition rather than
treating absent results as eligible; preserve the base_images and manifest_match
checks and ensure clean Go scans remain absent. Apply the same single-module
condition used by determine_verdict.
- Around line 209-219: Update find_go_module_dirs to use os.walk instead of
repo_dir.rglob("go.mod"), removing GO_MOD_SKIP_DIRS entries from each directory
list before descending so skipped trees such as .git, vendor, and node_modules
are never traversed. Preserve resolved paths, module-root collection, and
repo-root-first sorting.
- Around line 473-483: Update scan_go_repository to capture each module’s
toolchain_matched value while building module_results, retain the primary
module’s value, and return it instead of hardcoded None in the result
dictionary. Preserve existing per-module scanning and ensure main continues
receiving the primary toolchain mismatch signal.
- Around line 486-494: Update the help text for check-manifest and the guidance
in skills/report.md to document that root Go repositories may return a
semicolon-joined manifest summary containing matches from multiple go.mod files.
Describe the output as match evidence rather than a single manifest line, and
instruct callers to interpret it accordingly; leave check_manifests behavior
unchanged.

In `@cve-fix/scripts/test_scan.py`:
- Line 89: Update the path assertions in cve-fix/scripts/test_scan.py at lines
89-89, 101-101, 154-154, and 187-188 to compare resolved paths: use
tools.resolve(), proxy.resolve(), repo.resolve(), vendor_parent.resolve(), and
tools.resolve() respectively. You may resolve each temporary repository path
once per test, such as through a resolved repo variable, and reuse it across the
assertions.
- Around line 59-68: Add tests in test_tool_module_verdict to cover the empty
manifest_line early return, the no-fixed_version fallback, and the scan_failed
result when no version resolves. Use representative inputs that exercise each
branch in tool_module_verdict and assert the exact expected verdict, including
the manual-review scan_failed outcome.
- Around line 22-57: Add assertions to test_compare_go_versions_prerelease
covering both _prerelease_less branches: compare prerelease identifiers with
different lengths so the shorter list is vulnerable/lower precedence, and
compare a numeric identifier with an alphanumeric identifier so the numeric form
is vulnerable/lower precedence. Use valid versions with the same core version
and opposite-order assertions where needed.
- Around line 103-120: Add an assertion to
test_resolve_language_explicit_override covering an empty or whitespace-only
LANGUAGE environment value, and verify resolve_language(repo) falls back to
auto-detection and returns "node", matching the documented behavior and existing
.strip() handling.
- Around line 10-18: Add cve-fix/scripts/test_scan.py to the CI test-discovery
configuration alongside _shared/scripts so the workflow executes it, preserving
the existing test discovery behavior for other directories.

In `@cve-fix/skills/scan.md`:
- Around line 169-174: Make the “Per-Module Results (Go multi-module repos)”
section conditional on the presence of modules_scanned in scan-result.json, so
Node, Python, and subdirectory Go scans do not produce an empty table. Match the
neighbouring VEX section’s “(if applicable)” convention while preserving the
existing table for repository-wide Go scans.
- Around line 48-61: Update the scan instructions around FIXED_VERSION to state
that it is honored only when build_location is the default "."; when scanning a
subdirectory, instruct users to omit it because scan.py ignores it there.
Preserve the existing guidance to omit FIXED_VERSION when no fixed version is
known.
- Around line 121-123: Update the modules_scanned guidance to replace
“unaffected” with the valid verdict value “absent,” and state that VEX closure
is appropriate only when every module is patched, absent, or informational.
- Around line 40-46: Update the scan workflow command around ../scripts/scan.py
to make the scanner path independent of the shell’s current working directory.
Run the command from the workflow root or resolve the script path relative to
the workflow location before invoking scan.py, while preserving the existing
arguments and environment variables.

In `@cve-fix/skills/start.md`:
- Around line 161-163: The ecosystem schema must use the same primary marker
between the producer and consumer: in cve-fix/skills/start.md lines 161-163,
retain the [primary] suffix and state that /scan reads it to select LANGUAGE; in
cve-fix/skills/scan.md lines 27-36, instruct the agent to use the ecosystem
tagged [primary] in context.md and re-derive it from the ticket only when the
marker is absent.
- Around line 109-117: Clarify the ecosystem-detection instructions around the
Go module inventory and primary ecosystem selection: use the
runtime-versus-dev-tooling heuristic defined in scan.md, and when no ecosystem
matches the affected package, multiple ecosystems plausibly match, or the ticket
lacks package context, stop and ask the user which ecosystem is primary before
recording it for LANGUAGE. Require recording the user’s answer and preserve the
existing all-ecosystems listing.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a1d7b5dc-cf67-47a2-a973-0bbae30b6522

📥 Commits

Reviewing files that changed from the base of the PR and between a605aa5 and a13ff32.

📒 Files selected for processing (5)
  • cve-fix/SKILL.md
  • cve-fix/scripts/scan.py
  • cve-fix/scripts/test_scan.py
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
**/{SKILL.md,guidelines.md,skills/*.md,commands/*.md}

📄 CodeRabbit inference engine (Custom checks)

Flag any absolute filesystem path in markdown files within workflow directories (*/SKILL.md, /skills/.md, /commands/.md, */guidelines.md). Paths like /home/, /Users/, /tmp/, /var/, /opt/ are prohibited because workflows are installed via symlink and must use relative paths only. Paths inside fenced code blocks that are clearly examples (containing "example", "e.g.", or placeholder usernames like /home/user/) are exempt.

Files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
**/{SKILL.md,guidelines.md,controller.md}

📄 CodeRabbit inference engine (Custom checks)

When any of SKILL.md, guidelines.md, or controller.md in a workflow is changed, compare it against whichever of the other two files are present and check for verbatim duplication of multi-line instruction blocks or paragraphs. Each has a distinct role: SKILL.md is the thin entry point, guidelines.md holds principles/limits/safety/quality/escalation, controller.md manages phase dispatch. Phase names and brief one-line descriptions appearing in multiple files is EXPECTED (cross-referencing, not duplication) — only flag substantial blocks of identical prose or step-by-step instructions that are copied between files.

Files:

  • cve-fix/SKILL.md
**/*.md

📄 CodeRabbit inference engine (Custom checks)

For any changed markdown file in a workflow directory, verify that file path references (backtick-quoted paths like ../skills/controller.md or guidelines.md) point to files that exist. Flag references to files that don't exist (dangling references). Also flag skill or command files that exist but are never referenced from SKILL.md, controller.md, or any command file (orphaned files).

Files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md

⚙️ CodeRabbit configuration file

**/*.md: Cross-workflow consistency (ai-workflows conventions):

  • All file references must be relative paths (never absolute) —
    this is critical for symlink compatibility
  • No IDE-specific syntax (Cursor-specific, VS Code-specific, etc.)
  • Consistent terminology within a workflow: pick one term, stick
    with it
  • Schema field names and types must match between producer and
    consumer files (e.g., if a field is defined in one phase skill
    and consumed in another, names and types must agree)
  • No verbatim duplication of multi-line instruction blocks
    across SKILL.md, guidelines.md, and controller.md — each has
    a distinct role (shared phase names and brief references are
    expected cross-referencing, not duplication)

Files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
**/SKILL.md

📄 CodeRabbit inference engine (Custom checks)

For any SKILL.md file changed in this PR, verify it is under 30 lines total (including frontmatter). SKILL.md must be thin entry points using progressive disclosure. If a SKILL.md exceeds 30 lines, flag it with the count and suggest moving content to guidelines.md or skills/ files.

**/SKILL.md: Every workflow must have a SKILL.md entry point with YAML frontmatter containing name, version, and description.
Keep SKILL.md under 30 lines and place detailed behavior in guidelines.md and skills/.
SKILL.md may reference guidelines.md and optionally skills/controller.md using same-directory relative paths.
Workflows must not auto-advance in attended mode unless an explicit unattended mode is documented.
When behavioral workflow files change, update the corresponding workflow version in SKILL.md according to semver: patch for wording or formatting, minor for behavioral or phase changes, and major for removed, renamed, or restructured phases.
Do not bump versions for non-behavioral files such as README.md and GUIDE.md.

Files:

  • cve-fix/SKILL.md

⚙️ CodeRabbit configuration file

**/SKILL.md: SKILL.md review (ai-workflows conventions):

  • YAML frontmatter required: opening/closing --- delimiters
  • Required fields: name (lowercase, hyphens only, max 64 chars),
    description (third person, includes trigger terms and
    activated-by commands)
  • Total file length must be under 30 lines (progressive
    disclosure rule — details belong in guidelines.md or skills/)
  • Must reference guidelines.md for principles/limits/safety/quality
  • Must NOT duplicate content from guidelines.md or controller.md
  • Should list all phases with references to skills/ or commands/
  • No IDE-specific syntax — plain markdown only
  • Verify every file path reference resolves to an existing file

Files:

  • cve-fix/SKILL.md
**/*.{md,py,sh}

📄 CodeRabbit inference engine (AGENTS.md)

Workflow content must use plain markdown and contain no IDE-specific syntax.

Files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/scripts/test_scan.py
  • cve-fix/scripts/scan.py
  • cve-fix/skills/start.md
**/*

📄 CodeRabbit inference engine (AGENTS.md)

Save all significant workflow outputs under .artifacts/{workflow-name}/{context}/.

Files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/scripts/test_scan.py
  • cve-fix/scripts/scan.py
  • cve-fix/skills/start.md
**/skills/*.md

📄 CodeRabbit inference engine (Custom checks)

For any changed skills/*.md file, verify that main steps are numbered sequentially (Step 1, Step 2, Step 3... or ## Step 1, ## Step 2...). Flag: gaps in numbering (1, 2, 4), duplicate numbers (two Step 3s), and any skill with more than 10 main steps (cognitive load risk for AI agents). Sub-steps (Step 1a, Step 3b) are acceptable ONLY when they represent conditional branches off the parent step (e.g., "Step 1a: If , do X"). Flag sub-steps that are actually new main steps inserted to avoid renumbering — those should be promoted to full steps with the sequence renumbered.

When workflows invoke commands that can affect shared systems, follow the documented approval and safety gates: verify git status before destructive Git operations, confirm branch and base before PR/MR pushes, require explicit approval for Jira writes, and run Vale before applying documentation changes.

Files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md

⚙️ CodeRabbit configuration file

**/skills/*.md: Phase skill review (ai-workflows conventions):

  • Maximum 10 steps per skill invocation — flag if exceeded
    (cognitive load / context window risk for AI agents)
  • Main steps must be numbered sequentially: no gaps, no
    duplicates. Sub-steps (e.g., Step 1a) are allowed ONLY for
    conditional branches off a parent step — never as a way to
    insert a new main step without renumbering
  • Internal cross-references (e.g., "see Step 4") must point to
    correct step numbers
  • No step should depend on output from a later step
  • Synthesis tasks (summarization, assessment, verdict) must NOT
    be buried after heavy per-item processing — they degrade in
    long contexts
  • controller.md must reference sibling skills as phase-name.md
    (not skills/phase-name.md) — relative to its own directory
  • Skills referencing _shared/ resources must use the correct
    relative path depth (e.g., ../../_shared/recipes/self-review-gate.md
    from skills/)
  • Failure modes must be documented: what to do when prerequisites
    are missing, when zero results are returned, when tools are
    unavailable
  • Escalation criteria must be clear: when to stop and ask the user
  • Instructions must be unambiguous — an AI agent reading
    top-to-bottom should produce correct output on the first try
  • If the file has YAML frontmatter, name and description are required

Files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
**/{SKILL,guidelines,README,skills,commands,templates,prompts}/*

📄 CodeRabbit inference engine (AGENTS.md)

Use relative paths for all file references to preserve symlink compatibility across installation scopes.

Files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
**/{SKILL,guidelines,skills,commands,templates,prompts}/*

📄 CodeRabbit inference engine (AGENTS.md)

Behavioral changes must include the version bump in the same commit; do not create a separate version-bump commit.

Files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
**/scripts/*.py

⚙️ CodeRabbit configuration file

**/scripts/*.py: Workflow script review (ai-workflows conventions):

  • Scripts must be invoked by skill files, not by users directly
  • Must work when the workflow is installed via symlink
  • Exit code conventions must be documented in docstring:
    Report scripts: 0 = informational, 1 = halt
    Search/query scripts: define semantics in docstring
  • Python 3 required; no Python 2 compatibility needed
  • No hardcoded absolute paths — derive paths relative to
    script location

Files:

  • cve-fix/scripts/test_scan.py
  • cve-fix/scripts/scan.py
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}

⚙️ CodeRabbit configuration file

**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):

  • SQL: parameterized queries only; no string concatenation
  • Command: no shell=True, os.system, or backtick exec with user input
  • LDAP/XPath: escape special characters in filters
  • Path traversal: canonicalize paths, reject ../
  • Deserialization: no pickle/yaml.load()/eval on untrusted data
  • Prototype pollution: no recursive merge of untrusted objects
  • Validate at trust boundaries with allow-lists, not deny-lists
  • Normalize Unicode and anchor regexes (^$); watch for ReDoS

Files:

  • cve-fix/scripts/test_scan.py
  • cve-fix/scripts/scan.py
🧠 Learnings (8)
📚 Learning: 2026-04-15T10:18:31.948Z
Learnt from: galel12
Repo: flightctl/ai-workflows PR: 22
File: kcs/templates/section-guidance.md:73-73
Timestamp: 2026-04-15T10:18:31.948Z
Learning: When reviewing Markdown workflow skill docs in these directories (e.g., docs-writer, cve-fix, kcs), treat `issues.redhat.com` as an intentional, repo-wide convention for example Jira URLs. Do not flag it as an unwanted hardcoded/project-specific host or as a secret/sensitive value solely because it appears in the URL.

Applied to files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-06-15T15:50:50.503Z
Learnt from: adalton
Repo: flightctl/ai-workflows PR: 64
File: skill-reviewer/SKILL.md:3-3
Timestamp: 2026-06-15T15:50:50.503Z
Learning: In flightctl/ai-workflows, treat `SKILL.md` as a size-constrained document: keep it at or under 30 lines. If a `SKILL.md` already exceeds 30 lines but was not changed by the current PR (a known pre-existing issue), don’t require fixing it as part of the PR. If the PR does modify a too-long `SKILL.md`, refactor it into a thin entry point (e.g., move bulk content to smaller companion docs and leave only a brief overview/links) so the `SKILL.md` itself stays within the 30-line limit.

Applied to files:

  • cve-fix/SKILL.md
📚 Learning: 2026-07-23T14:18:59.204Z
Learnt from: adalton
Repo: flightctl/ai-workflows PR: 84
File: bugfix/SKILL.md:3-3
Timestamp: 2026-07-23T14:18:59.204Z
Learning: In flightctl/ai-workflows documentation, treat backtick-quoted workflow path templates that include placeholders (e.g., `commands/{command}.md`, `skills/{phase}.md`) as runtime-dispatch/template instructions for AI agents, not literal Markdown links. When these appear, do not flag them as dangling/invalid references solely because the braces indicate substitution of an invoked command or phase name at runtime.

Applied to files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-08-06T13:07:53.827Z
Learnt from: asafbennatan
Repo: flightctl/ai-workflows PR: 99
File: pr-review/skills/start.md:0-0
Timestamp: 2026-08-06T13:07:53.827Z
Learning: In Markdown templates containing nested triple-backtick code fences, wrap the outer template block with a fence of at least four backticks. This prevents inner triple-backtick fences from prematurely terminating the outer block and preserves correct Markdown rendering.

Applied to files:

  • cve-fix/SKILL.md
  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-04-12T00:25:51.234Z
Learnt from: adalton
Repo: flightctl/ai-workflows PR: 20
File: design/skills/respond.md:29-31
Timestamp: 2026-04-12T00:25:51.234Z
Learning: In flightctl/ai-workflows skill markdown files, treat path references as two categories:
1) For cross-document markdown links (e.g., links to other .md files like ../skills/controller.md or ../../templates/design.md), use paths relative to the current markdown file’s location so links work under symlinks.
2) For runtime artifact paths used as prose instructions to the AI agent (e.g., .artifacts/design/{issue-number}/publish-metadata.json or .artifacts/prd/config.json), keep them repo-root-relative (start with .artifacts/). Do not convert these artifact paths to be relative to the skill file directory (e.g., don’t rewrite to ../../.artifacts/...), because the AI resolves them from the repo root.

Applied to files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-04-15T10:19:54.839Z
Learnt from: galel12
Repo: flightctl/ai-workflows PR: 22
File: kcs/skills/gather.md:25-26
Timestamp: 2026-04-15T10:19:54.839Z
Learning: In flightctl/ai-workflows, for Jira URL examples inside skill Markdown files, follow the repo-wide convention and use a real example Jira link of the form `https://issues.redhat.com/browse/PROJ-123` (not a generic placeholder like `https://example.com/...`). Since this is a documented convention, do not flag it as a portability/documentation hardcoding issue when reviewing similar skill markdown files.

Applied to files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-04-16T10:39:50.418Z
Learnt from: galel12
Repo: flightctl/ai-workflows PR: 22
File: kcs/skills/gather.md:34-37
Timestamp: 2026-04-16T10:39:50.418Z
Learning: In flightctl/ai-workflows workflow skill files (e.g., kcs/bugfix/prd/design skills), do not require sanitization/normalization of free-form user-supplied identifier placeholders (such as {issue-key} or {issue-number}) when they’re used to construct artifact paths like `.artifacts/{workflow}/{identifier}/`. This is intentional because these workflows run in human-supervised IDE sessions where the user provides the values interactively and confirms the output. Therefore, do not flag missing sanitization/normalization of these identifiers as a security or correctness issue during review for these skill files.

Applied to files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
📚 Learning: 2026-05-25T17:11:32.207Z
Learnt from: galel12
Repo: flightctl/ai-workflows PR: 47
File: README.md:140-142
Timestamp: 2026-05-25T17:11:32.207Z
Learning: In markdown files under the repo’s skill/command areas (e.g., `skills/**` and `commands/**`), any references to other files on disk (like links/includes pointing to other skill/command markdown such as `../skills/controller.md` or `commands/*.md`) must use relative paths—never absolute paths (no leading `/` or fully-qualified filesystem paths). This ensures the references remain symlink-safe and resolve correctly at runtime. Do not apply this rule to human-facing prose docs like `README.md`/`CONTRIBUTING.md`; when those documents intentionally distinguish user-level vs project-level install locations, keep the absolute user-level paths (e.g., `~/.cursor/commands/`) as written so the distinction is clear.

Applied to files:

  • cve-fix/skills/scan.md
  • cve-fix/skills/start.md
🪛 LanguageTool
cve-fix/skills/scan.md

[typographical] ~68-~68: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...der .git, vendor, and node_modules. The JSON output includes a `modules_sca...

(WRB_QUESTION_MARK)


[style] ~75-~75: Consider using the typographical ellipsis character here instead.
Context: ...o.mod| Application/runtime binaries |govulncheck ./...| |tools/go.mod | Dev tools (mockg...

(ELLIPSIS)

🪛 Ruff (0.16.1)
cve-fix/scripts/test_scan.py

[warning] 127-127: Use pytest.raises instead of unittest-style assertRaises

Replace assertRaises with pytest.raises

(PT027)

cve-fix/scripts/scan.py

[warning] 93-95: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (13)
cve-fix/scripts/scan.py (7)

22-29: LGTM!


50-61: LGTM!


63-96: LGTM!


229-258: LGTM!


294-371: LGTM!


374-403: LGTM!


706-708: LGTM!

Also applies to: 799-800

cve-fix/scripts/test_scan.py (1)

70-78: LGTM!

cve-fix/skills/start.md (2)

89-89: LGTM!


172-174: LGTM!

cve-fix/skills/scan.md (2)

63-81: LGTM!


103-105: LGTM!

cve-fix/SKILL.md (1)

3-3: 📐 Maintainability & Code Quality

No change required. cve-fix/SKILL.md has 25 lines. The previous version was 0.4.0, so 0.5.0 is a valid minor increment.

Comment thread cve-fix/scripts/scan.py
Comment on lines +209 to +219
def find_go_module_dirs(repo_dir: Path) -> list[Path]:
"""Find Go module roots under repo_dir, with the repo root listed first."""
repo_dir = repo_dir.resolve()
modules: list[Path] = []
for gomod in sorted(repo_dir.rglob("go.mod")):
relative_parts = gomod.relative_to(repo_dir).parts[:-1]
if any(part in GO_MOD_SKIP_DIRS for part in relative_parts):
continue
modules.append(gomod.parent)
modules.sort(key=lambda path: (path.resolve() != repo_dir, str(path)))
return modules

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Prune skipped directories during traversal instead of after.

rglob("go.mod") walks the complete tree, including .git, vendor, and node_modules, and discards the matches afterwards. This PR targets polyglot repositories such as flightctl-ui, where node_modules can hold tens of thousands of entries. find_go_module_dirs also runs more than once per invocation: scan_go_repository calls it at Line 409, check_all_go_manifests calls it again at Line 248, and check_manifests can call it a third time.

Use os.walk and prune the skip directories in place.

♻️ Pruned traversal
 def find_go_module_dirs(repo_dir: Path) -> list[Path]:
     """Find Go module roots under repo_dir, with the repo root listed first."""
     repo_dir = repo_dir.resolve()
     modules: list[Path] = []
-    for gomod in sorted(repo_dir.rglob("go.mod")):
-        relative_parts = gomod.relative_to(repo_dir).parts[:-1]
-        if any(part in GO_MOD_SKIP_DIRS for part in relative_parts):
-            continue
-        modules.append(gomod.parent)
+    for dirpath, dirnames, filenames in os.walk(repo_dir):
+        dirnames[:] = [d for d in dirnames if d not in GO_MOD_SKIP_DIRS]
+        if "go.mod" in filenames:
+            modules.append(Path(dirpath))
     modules.sort(key=lambda path: (path.resolve() != repo_dir, str(path)))
     return modules
📝 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.

Suggested change
def find_go_module_dirs(repo_dir: Path) -> list[Path]:
"""Find Go module roots under repo_dir, with the repo root listed first."""
repo_dir = repo_dir.resolve()
modules: list[Path] = []
for gomod in sorted(repo_dir.rglob("go.mod")):
relative_parts = gomod.relative_to(repo_dir).parts[:-1]
if any(part in GO_MOD_SKIP_DIRS for part in relative_parts):
continue
modules.append(gomod.parent)
modules.sort(key=lambda path: (path.resolve() != repo_dir, str(path)))
return modules
def find_go_module_dirs(repo_dir: Path) -> list[Path]:
"""Find Go module roots under repo_dir, with the repo root listed first."""
repo_dir = repo_dir.resolve()
modules: list[Path] = []
for dirpath, dirnames, filenames in os.walk(repo_dir):
dirnames[:] = [d for d in dirnames if d not in GO_MOD_SKIP_DIRS]
if "go.mod" in filenames:
modules.append(Path(dirpath))
modules.sort(key=lambda path: (path.resolve() != repo_dir, str(path)))
return modules
🤖 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 `@cve-fix/scripts/scan.py` around lines 209 - 219, Update find_go_module_dirs
to use os.walk instead of repo_dir.rglob("go.mod"), removing GO_MOD_SKIP_DIRS
entries from each directory list before descending so skipped trees such as
.git, vendor, and node_modules are never traversed. Preserve resolved paths,
module-root collection, and repo-root-first sorting.

Comment thread cve-fix/scripts/scan.py
Comment on lines +273 to +291
def scan_go_tool_module(mod_dir: Path, package: str) -> dict:
"""Scan a tool-only Go module where govulncheck has no packages to analyze."""
manifest_line = check_single_go_manifest(mod_dir, package)
version, err = resolve_go_package_version(mod_dir, package)
output_parts = [
f"Module: {mod_dir}",
f"Manifest: {manifest_line or 'not found'}",
f"Resolved version: {version or err or 'unknown'}",
"Note: govulncheck not applicable (tool-only module)",
]
return {
"scan_tool": "go_list_m",
"scan_exit_code": 0 if version else 1,
"scan_output": "\n".join(output_parts),
"toolchain_matched": None,
"target_go_version": extract_go_version(mod_dir) or None,
"resolved_version": version or None,
"manifest_line": manifest_line,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Report the module directory relative to the repository.

f"Module: {mod_dir}" writes the absolute filesystem path into scan_output. That string flows into combined_output and then into scan-result.json, so the artifact records the machine-local checkout path. Pass the relative module directory that scan_go_repository already computes at Line 426, or accept a display name parameter.

🤖 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 `@cve-fix/scripts/scan.py` around lines 273 - 291, Update scan_go_tool_module
so its “Module” output uses the repository-relative module directory rather than
the absolute mod_dir path. Reuse the relative directory already computed by
scan_go_repository and pass it into the function, or add an equivalent
display-name parameter while preserving the existing scan result behavior.

Comment thread cve-fix/scripts/scan.py
Comment on lines +424 to +434
for mod_dir in modules:
rel = mod_dir.resolve().relative_to(repo_dir.resolve())
module_dir = "." if rel == Path(".") else str(rel)
manifest_line = check_single_go_manifest(mod_dir, package)

scan_result = scan_go(mod_dir, cve_id, package)
if (
not is_successful_scan(scan_result["scan_exit_code"], "govulncheck")
and _is_tool_only_go_failure(scan_result["scan_output"])
):
scan_result = scan_go_tool_module(mod_dir, package)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Per-module scanning multiplies the worst-case scan duration by the module count.

scan_go uses the full SCAN_TIMEOUT for each run call and retries up to four times per module (Lines 136-158). Running it once per discovered module makes the worst case module_count * 4 * SCAN_TIMEOUT. With the documented 300 s default and five modules, that reaches roughly 100 minutes. The previous single-module path was bounded at four calls.

cve-fix/skills/scan.md Lines 63-69 tell the user that every go.mod is scanned, but state no time budget. Divide the budget across modules, or document the expected duration in cve-fix/skills/scan.md.

🤖 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 `@cve-fix/scripts/scan.py` around lines 424 - 434, The per-module scan loop can
multiply the total timeout by the number of modules. Update the scanning flow
around scan_go and scan_go_tool_module to enforce a bounded overall budget, such
as dividing the available SCAN_TIMEOUT across discovered modules while
preserving retry behavior, or document the resulting worst-case duration in the
scan guidance; ensure the documented behavior matches the implemented time
budget.

Comment thread cve-fix/scripts/scan.py
Comment on lines +473 to +483
return {
"language": "go",
"verdict": overall_verdict,
"scan_tool": primary.get("scan_tool"),
"scan_exit_code": primary.get("scan_exit_code"),
"scan_output": combined_output,
"toolchain_matched": None,
"target_go_version": extract_go_version(modules[primary_idx]) or None,
"manifest_match": manifest_summary,
"modules_scanned": module_results,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

toolchain_matched is hardcoded to None and discards the primary module result.

scan_go records real toolchain_matched values (Lines 130, 150, 164). scan_go_repository drops them and always returns None. main copies that value into the result JSON at Line 748, and cve-fix/skills/scan.md Step 5 renders Toolchain matched: {yes/no/n/a}.

Every root Go repository scan now reports n/a. The toolchain-mismatch signal exists to prevent false negatives, and cve-fix/skills/scan.md Line 95 instructs the agent to note the mismatch. Propagate the primary module value instead.

Note: module_results also does not retain toolchain_matched, so the value must be captured in the loop.

🐛 Propagate the primary module toolchain result
         module_results.append({
             "module_dir": module_dir,
             "manifest_path": _manifest_path(repo_dir, mod_dir),
             "manifest_line": manifest_line,
             "resolved_version": scan_result.get("resolved_version"),
             "scan_tool": scan_result.get("scan_tool"),
             "scan_exit_code": scan_result.get("scan_exit_code"),
+            "toolchain_matched": scan_result.get("toolchain_matched"),
             "verdict": module_verdict,
         "scan_output": combined_output,
-        "toolchain_matched": None,
+        "toolchain_matched": primary.get("toolchain_matched"),
         "target_go_version": extract_go_version(modules[primary_idx]) or 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 `@cve-fix/scripts/scan.py` around lines 473 - 483, Update scan_go_repository to
capture each module’s toolchain_matched value while building module_results,
retain the primary module’s value, and return it instead of hardcoded None in
the result dictionary. Preserve existing per-module scanning and ensure main
continues receiving the primary toolchain mismatch signal.

Comment thread cve-fix/scripts/scan.py
Comment on lines +486 to +494
def check_manifests(work_dir: Path, lang: str, package: str,
repo_dir: Path | None = None) -> str:
"""Check if the package appears in any manifest file (case-insensitive)."""
if lang == "go" and repo_dir is not None and work_dir.resolve() == repo_dir.resolve():
matches = check_all_go_manifests(repo_dir, package)
if not matches:
return ""
return "; ".join(f"{match['manifest_path']}: {match['line']}" for match in matches)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find consumers of the check-manifest subcommand and of manifest_match.
rg -n -C4 'check-manifest' --glob '!**/.git/**'
rg -n -C3 'manifest_match' --glob '!**/.git/**'

Repository: flightctl/ai-workflows

Length of output: 160


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(scan\.py|.*\.(md|py|sh|yml|yaml))$' | head -200
printf '%s\n' '--- scan.py relevant structure ---'
scan_file="$(git ls-files | rg '(^|/)cve-fix/scripts/scan\.py$' | head -1)"
if [ -z "$scan_file" ]; then
  scan_file="$(git ls-files | rg '(^|/)scan\.py$' | head -1)"
fi
printf 'file=%s\n' "$scan_file"
wc -l "$scan_file"
rg -n -C5 'def check_manifests|def check_manifest_command|check_manifest|manifest_match|check-manifest|check_all_go_manifests|repo_dir' "$scan_file"
printf '%s\n' '--- repository-wide symbol consumers ---'
rg -n -C3 'check_manifest_command|check_manifests|manifest_match|check-manifest|check_all_go_manifests' --glob '!**/.git/**' . || true

Repository: flightctl/ai-workflows

Length of output: 33037


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- report skill usage ---'
sed -n '260,315p' cve-fix/skills/report.md
printf '%s\n' '--- manifest-related tests ---'
rg -n -C5 'check_manifests|check_manifest_command|check-manifest|manifest_match|multiple|multi' cve-fix/scripts/test_scan.py cve-fix/skills cve-fix/commands cve-fix/SKILL.md cve-fix/guidelines.md || true
printf '%s\n' '--- implementation context ---'
sed -n '209,260p' cve-fix/scripts/scan.py
sed -n '486,530p' cve-fix/scripts/scan.py
sed -n '656,688p' cve-fix/scripts/scan.py

Repository: flightctl/ai-workflows

Length of output: 21725


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- complete report instructions for the command output ---'
sed -n '287,335p' cve-fix/skills/report.md
printf '%s\n' '--- static behavioral verifier ---'
python3 - <<'PY'
import ast
from pathlib import Path

path = Path("cve-fix/scripts/scan.py")
tree = ast.parse(path.read_text())
functions = {
    node.name: node for node in tree.body
    if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
}
check = functions["check_manifests"]
returns = [
    node for node in ast.walk(check)
    if isinstance(node, ast.Return)
]
joined = [
    node for node in returns
    if isinstance(node.value, ast.Call)
    and isinstance(node.value.func, ast.Attribute)
    and node.value.func.attr == "join"
]
print(f"check_manifests return count: {len(returns)}")
print(f"join-based returns: {len(joined)}")
if not joined:
    raise SystemExit("expected Go multi-manifest join return not found")

# Evaluate only the formatting expression represented by the source.
matches = [
    {"manifest_path": "go.mod", "line": "require example.test/pkg v1.0.0"},
    {"manifest_path": "tools/go.mod", "line": "require example.test/pkg v1.1.0"},
]
summary = "; ".join(f"{m['manifest_path']}: {m['line']}" for m in matches)
print(summary)
assert summary == (
    "go.mod: require example.test/pkg v1.0.0; "
    "tools/go.mod: require example.test/pkg v1.1.0"
)
PY

Repository: flightctl/ai-workflows

Length of output: 2784


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- complete report instructions for the command output ---'
sed -n '287,335p' cve-fix/skills/report.md
printf '%s\n' '--- static behavioral verifier ---'
python3 - <<'PY'
import ast
from pathlib import Path

path = Path("cve-fix/scripts/scan.py")
tree = ast.parse(path.read_text())
functions = {
    node.name: node for node in tree.body
    if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef))
}
check = functions["check_manifests"]
returns = [
    node for node in ast.walk(check)
    if isinstance(node, ast.Return)
]
joined = [
    node for node in returns
    if isinstance(node.value, ast.Call)
    and isinstance(node.value.func, ast.Attribute)
    and node.value.func.attr == "join"
]
print(f"check_manifests return count: {len(returns)}")
print(f"join-based returns: {len(joined)}")
if not joined:
    raise SystemExit("expected Go multi-manifest join return not found")

matches = [
    {"manifest_path": "go.mod", "line": "require example.test/pkg v1.0.0"},
    {"manifest_path": "tools/go.mod", "line": "require example.test/pkg v1.1.0"},
]
summary = "; ".join(f"{m['manifest_path']}: {m['line']}" for m in matches)
print(summary)
assert summary == (
    "go.mod: require example.test/pkg v1.0.0; "
    "tools/go.mod: require example.test/pkg v1.1.0"
)
PY

Repository: flightctl/ai-workflows

Length of output: 2784


Document the multi-manifest output contract.

For root Go repositories, check-manifest can print semicolon-joined matches from multiple go.mod files. Update the help text and cve-fix/skills/report.md to describe a manifest summary, not a single manifest line, and instruct callers to treat the output as match evidence.

🤖 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 `@cve-fix/scripts/scan.py` around lines 486 - 494, Update the help text for
check-manifest and the guidance in skills/report.md to document that root Go
repositories may return a semicolon-joined manifest summary containing matches
from multiple go.mod files. Describe the output as match evidence rather than a
single manifest line, and instruct callers to interpret it accordingly; leave
check_manifests behavior unchanged.

Comment thread cve-fix/skills/scan.md
Comment on lines +48 to +61
When a fixed version is known from `context.md` (for example `0.52.0`),
also set `FIXED_VERSION` so secondary Go modules (such as `tools/go.mod`)
can be compared by version:

```bash
OUTPUT_DIR=.artifacts/cve-fix/{context} \
LANGUAGE={language} \
FIXED_VERSION={fixed_version} \
python3 ../scripts/scan.py {repo_dir} {CVE_ID} {package} {build_location}
```

Omit `FIXED_VERSION` (or leave it empty) when no fixed version is known.
Omit `LANGUAGE` only when context has no usable ecosystem — the script then
falls back to auto-detection.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

State that FIXED_VERSION applies only to the default build_location.

cve-fix/scripts/scan.py reads FIXED_VERSION at Line 742, inside the branch guarded by lang == "go" and work_dir.resolve() == repo_dir.resolve(). When build_location is anything other than ., that branch does not run and the variable is ignored with no message.

Line 66 states the build_location condition for multi-module scanning but not for FIXED_VERSION. An agent that sets both a subdirectory build_location and FIXED_VERSION gets a silent no-op and may then read a scan_failed verdict as a real failure.

📝 Suggested addition
 Omit `FIXED_VERSION` (or leave it empty) when no fixed version is known.
+`FIXED_VERSION` applies only when `build_location` is `.`, because the scanner
+compares versions during the repository-wide Go module scan.
 Omit `LANGUAGE` only when context has no usable ecosystem — the script then
 falls back to auto-detection.
📝 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.

Suggested change
When a fixed version is known from `context.md` (for example `0.52.0`),
also set `FIXED_VERSION` so secondary Go modules (such as `tools/go.mod`)
can be compared by version:
```bash
OUTPUT_DIR=.artifacts/cve-fix/{context} \
LANGUAGE={language} \
FIXED_VERSION={fixed_version} \
python3 ../scripts/scan.py {repo_dir} {CVE_ID} {package} {build_location}
```
Omit `FIXED_VERSION` (or leave it empty) when no fixed version is known.
Omit `LANGUAGE` only when context has no usable ecosystem — the script then
falls back to auto-detection.
When a fixed version is known from `context.md` (for example `0.52.0`),
also set `FIXED_VERSION` so secondary Go modules (such as `tools/go.mod`)
can be compared by 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 `@cve-fix/skills/scan.md` around lines 48 - 61, Update the scan instructions
around FIXED_VERSION to state that it is honored only when build_location is the
default "."; when scanning a subdirectory, instruct users to omit it because
scan.py ignores it there. Preserve the existing guidance to omit FIXED_VERSION
when no fixed version is known.

Comment thread cve-fix/skills/scan.md
Comment on lines +121 to +123
- If `modules_scanned` is present: review each module independently. A VEX
closure is only appropriate when **all** modules are patched, unaffected,
or informational (vulnerable symbols not reachable)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

unaffected is not a verdict value.

The verdict vocabulary in the Step 3 table at Lines 109-116 is present, present_by_version, absent, in_base_image, informational, and scan_failed. VERDICT_PRIORITY in cve-fix/scripts/scan.py Lines 53-60 uses the same six values. Line 122 introduces "unaffected", which matches no module verdict in modules_scanned.

Use absent. State the condition against the actual values so the agent can evaluate it.

📝 Suggested wording
 - If `modules_scanned` is present: review each module independently. A VEX
-  closure is only appropriate when **all** modules are patched, unaffected,
-  or informational (vulnerable symbols not reachable)
+  closure is only appropriate when **every** module verdict is `absent` or
+  `informational` (vulnerable symbols not reachable)

As per path instructions: "Consistent terminology within a workflow: pick one term, stick with it".

📝 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.

Suggested change
- If `modules_scanned` is present: review each module independently. A VEX
closure is only appropriate when **all** modules are patched, unaffected,
or informational (vulnerable symbols not reachable)
- If `modules_scanned` is present: review each module independently. A VEX
closure is only appropriate when **every** module verdict is `absent` or
`informational` (vulnerable symbols not reachable)
🤖 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 `@cve-fix/skills/scan.md` around lines 121 - 123, Update the modules_scanned
guidance to replace “unaffected” with the valid verdict value “absent,” and
state that VEX closure is appropriate only when every module is patched, absent,
or informational.

Source: Path instructions

Comment thread cve-fix/skills/scan.md
Comment on lines +169 to +174
## Per-Module Results (Go multi-module repos)

| Module | Manifest | Resolved Version | Verdict |
|--------|----------|------------------|---------|
| {module_dir} | {manifest_path} | {resolved_version} | {verdict} |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Mark the per-module section conditional.

modules_scanned appears in scan-result.json only for the repository-wide Go path (cve-fix/scripts/scan.py Lines 799-800). For Node, Python, and subdirectory Go scans the key is absent, and the agent writes an empty table. The neighbouring VEX section at Line 175 already uses the "(if applicable)" convention.

📝 Suggested wording
-## Per-Module Results (Go multi-module repos)
+## Per-Module Results (if `modules_scanned` is present)
+
+Omit this section when the scan result has no `modules_scanned` array.
📝 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.

Suggested change
## Per-Module Results (Go multi-module repos)
| Module | Manifest | Resolved Version | Verdict |
|--------|----------|------------------|---------|
| {module_dir} | {manifest_path} | {resolved_version} | {verdict} |
## Per-Module Results (if `modules_scanned` is present)
Omit this section when the scan result has no `modules_scanned` array.
| Module | Manifest | Resolved Version | Verdict |
|--------|----------|------------------|---------|
| {module_dir} | {manifest_path} | {resolved_version} | {verdict} |
🤖 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 `@cve-fix/skills/scan.md` around lines 169 - 174, Make the “Per-Module Results
(Go multi-module repos)” section conditional on the presence of modules_scanned
in scan-result.json, so Node, Python, and subdirectory Go scans do not produce
an empty table. Match the neighbouring VEX section’s “(if applicable)”
convention while preserving the existing table for repository-wide Go scans.

Comment thread cve-fix/skills/start.md
Comment on lines +109 to +117
For Go projects, list every `go.mod` discovered in the repository (for example
`go.mod`, `tools/go.mod`, `proxy/go.mod`), excluding paths under `.git`, `vendor`,
and `node_modules`, and note which module is runtime vs dev-tooling. Record
nested Go modules even when the repository root is Node or Python (polyglot).

When the repo is polyglot, list **all** ecosystems under Detected Ecosystem(s),
and mark the **primary** ecosystem for this CVE — the one that matches the
affected package from the ticket. `/scan` passes that primary value as
`LANGUAGE` so the scanner does not guess the wrong half of the repo.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Define what the agent does when the primary ecosystem is not determinable.

Line 115 tells the agent to mark the ecosystem "that matches the affected package from the ticket". Two outcomes have no rule:

  • No detected ecosystem matches the affected package. Example: the ticket names a Java package but the repository is Go plus Node.
  • More than one ecosystem plausibly matches, or the ticket does not name a package at all.

The chosen value becomes LANGUAGE for /scan (Lines 116-117), so a wrong guess sends the scanner at the wrong half of the repository, which is the exact false negative this change targets. cve-fix/skills/scan.md Lines 60-61 only cover the case where context has no usable ecosystem.

Line 111 has the same gap. "Note which module is runtime vs dev-tooling" states no criterion. cve-fix/skills/scan.md Lines 73-77 holds the heuristic table; reference it here or restate the rule.

Add an explicit escalation: if the primary ecosystem is ambiguous, ask the user and record the answer.

📝 Suggested addition
 For Go projects, list every `go.mod` discovered in the repository (for example
 `go.mod`, `tools/go.mod`, `proxy/go.mod`), excluding paths under `.git`, `vendor`,
 and `node_modules`, and note which module is runtime vs dev-tooling. Record
 nested Go modules even when the repository root is Node or Python (polyglot).
+A module is runtime when it builds shipped binaries or application packages. A
+module is dev-tooling when it only pins build-time tools (codegen, mocks, linters).
 
 When the repo is polyglot, list **all** ecosystems under Detected Ecosystem(s),
 and mark the **primary** ecosystem for this CVE — the one that matches the
 affected package from the ticket. `/scan` passes that primary value as
 `LANGUAGE` so the scanner does not guess the wrong half of the repo.
+
+If no detected ecosystem matches the affected package, or if more than one
+matches, stop and ask the user which ecosystem is primary. Record the answer in
+`context.md`. Do not guess.

As per path instructions: "Failure modes must be documented: what to do when prerequisites are missing, when zero results are returned" and "Escalation criteria must be clear: when to stop and ask the user".

📝 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.

Suggested change
For Go projects, list every `go.mod` discovered in the repository (for example
`go.mod`, `tools/go.mod`, `proxy/go.mod`), excluding paths under `.git`, `vendor`,
and `node_modules`, and note which module is runtime vs dev-tooling. Record
nested Go modules even when the repository root is Node or Python (polyglot).
When the repo is polyglot, list **all** ecosystems under Detected Ecosystem(s),
and mark the **primary** ecosystem for this CVE — the one that matches the
affected package from the ticket. `/scan` passes that primary value as
`LANGUAGE` so the scanner does not guess the wrong half of the repo.
For Go projects, list every `go.mod` discovered in the repository (for example
`go.mod`, `tools/go.mod`, `proxy/go.mod`), excluding paths under `.git`, `vendor`,
and `node_modules`, and note which module is runtime vs dev-tooling. Record
nested Go modules even when the repository root is Node or Python (polyglot).
A module is runtime when it builds shipped binaries or application packages. A
module is dev-tooling when it only pins build-time tools (codegen, mocks, linters).
When the repo is polyglot, list **all** ecosystems under Detected Ecosystem(s),
and mark the **primary** ecosystem for this CVE — the one that matches the
affected package from the ticket. `/scan` passes that primary value as
`LANGUAGE` so the scanner does not guess the wrong half of the repo.
If no detected ecosystem matches the affected package, or if more than one
matches, stop and ask the user which ecosystem is primary. Record the answer in
`context.md`. Do not guess.
🤖 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 `@cve-fix/skills/start.md` around lines 109 - 117, Clarify the
ecosystem-detection instructions around the Go module inventory and primary
ecosystem selection: use the runtime-versus-dev-tooling heuristic defined in
scan.md, and when no ecosystem matches the affected package, multiple ecosystems
plausibly match, or the ticket lacks package context, stop and ask the user
which ecosystem is primary before recording it for LANGUAGE. Require recording
the user’s answer and preserve the existing all-ecosystems listing.

Source: Path instructions

Comment thread cve-fix/skills/start.md
Comment on lines 161 to +163
## Detected Ecosystem(s)
- {ecosystem}: {package_manager} ({manifest_file})
- {ecosystem}: {package_manager} ({manifest_file}) [primary]
- {ecosystem}: {package_manager} ({manifest_file}) (additional, if polyglot)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

The [primary] ecosystem marker has no shared name between the producer and the consumer. /start records the primary ecosystem in context.md with a literal [primary] suffix, and /scan consumes "the primary ecosystem" without ever naming that marker. Each phase therefore derives the answer independently, and the two can disagree on the LANGUAGE value passed to the scanner.

  • cve-fix/skills/start.md#L161-L163: keep [primary] and state that /scan reads this marker to select LANGUAGE.
  • cve-fix/skills/scan.md#L27-L36: instruct the agent to read the ecosystem tagged [primary] in context.md, and to re-derive it from the ticket only when the marker is missing.

As per path instructions: "Schema field names and types must match between producer and consumer files".

📍 Affects 2 files
  • cve-fix/skills/start.md#L161-L163 (this comment)
  • cve-fix/skills/scan.md#L27-L36
🤖 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 `@cve-fix/skills/start.md` around lines 161 - 163, The ecosystem schema must
use the same primary marker between the producer and consumer: in
cve-fix/skills/start.md lines 161-163, retain the [primary] suffix and state
that /scan reads it to select LANGUAGE; in cve-fix/skills/scan.md lines 27-36,
instruct the agent to use the ecosystem tagged [primary] in context.md and
re-derive it from the ticket only when the marker is absent.

Source: Path instructions

@celdrake

Copy link
Copy Markdown
Author

Marking as Draft until the base PR merges, and I'll re-run the flow with Jira tickets rather than dependabot issues.

@celdrake
celdrake marked this pull request as draft August 10, 2026 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant