CVE fix: Dual ecosystem scan - #101
Conversation
Made-with: Cursor
Made-with: Cursor
WalkthroughThe 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. ChangesGo repository scanning
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| @@ -52,20 +61,41 @@ | |||
|
|
|||
|
|
|||
| def detect_language(work_dir: Path) -> str: | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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
📒 Files selected for processing (5)
cve-fix/SKILL.mdcve-fix/scripts/scan.pycve-fix/scripts/test_scan.pycve-fix/skills/scan.mdcve-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.mdcve-fix/skills/scan.mdcve-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.mdorguidelines.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.mdcve-fix/skills/scan.mdcve-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.mdcve-fix/skills/scan.mdcve-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 aSKILL.mdentry point with YAML frontmatter containingname,version, anddescription.
KeepSKILL.mdunder 30 lines and place detailed behavior inguidelines.mdandskills/.
SKILL.mdmay referenceguidelines.mdand optionallyskills/controller.mdusing 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 inSKILL.mdaccording 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 asREADME.mdandGUIDE.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.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.pycve-fix/skills/start.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Save all significant workflow outputs under
.artifacts/{workflow-name}/{context}/.
Files:
cve-fix/SKILL.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.pycve-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 statusbefore 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.mdcve-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.mdcve-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.mdcve-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.mdcve-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.pycve-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.pycve-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.mdcve-fix/skills/scan.mdcve-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.mdcve-fix/skills/scan.mdcve-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.mdcve-fix/skills/scan.mdcve-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.mdcve-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.mdcve-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.mdcve-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.mdcve-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 QualityNo change required.
cve-fix/SKILL.mdhas 25 lines. The previous version was0.4.0, so0.5.0is a valid minor increment.
| 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 |
There was a problem hiding this comment.
🚀 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.
| 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.
| 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, | ||
| } |
There was a problem hiding this comment.
📐 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.
| 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) |
There was a problem hiding this comment.
🩺 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.
| 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, | ||
| } |
There was a problem hiding this comment.
🗄️ 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.
| 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) | ||
|
|
There was a problem hiding this comment.
🗄️ 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/**' . || trueRepository: 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.pyRepository: 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"
)
PYRepository: 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"
)
PYRepository: 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.
| 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. |
There was a problem hiding this comment.
📐 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.
| 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.
| - 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) |
There was a problem hiding this comment.
📐 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.
| - 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
| ## Per-Module Results (Go multi-module repos) | ||
|
|
||
| | Module | Manifest | Resolved Version | Verdict | | ||
| |--------|----------|------------------|---------| | ||
| | {module_dir} | {manifest_path} | {resolved_version} | {verdict} | | ||
|
|
There was a problem hiding this comment.
📐 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.
| ## 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.
| 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. |
There was a problem hiding this comment.
📐 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.
| 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
| ## Detected Ecosystem(s) | ||
| - {ecosystem}: {package_manager} ({manifest_file}) | ||
| - {ecosystem}: {package_manager} ({manifest_file}) [primary] | ||
| - {ecosystem}: {package_manager} ({manifest_file}) (additional, if polyglot) |
There was a problem hiding this comment.
🗄️ 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/scanreads this marker to selectLANGUAGE.cve-fix/skills/scan.md#L27-L36: instruct the agent to read the ecosystem tagged[primary]incontext.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
|
Marking as Draft until the base PR merges, and I'll re-run the flow with Jira tickets rather than dependabot issues. |
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
scanmodule determines a single language based on the presence of certain files in the repository.For projects such as
flightctl-uiwhich 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.mdand is used later for scanning the CVE presence or absence.Example from flightctl-ui:
Node JS CVE
Go CVE
Workflows affected
startworkflow now detects nested Go modules and records primary and additional ecosystems incontext.md.scanworkflow now selects the CVE’s primary ecosystem throughLANGUAGE.Skills, commands, and guidelines
scan.pynow validates supported languages and detects nested Go modules.SKILL.mdversion changed from0.3.1to0.5.0.start.mdandscan.mdnow define polyglot repository handling and primary ecosystem reporting.Shared resources and conventions
_shared/resources.context.md,LANGUAGE, primary ecosystem selection, and repository context propagation.