cve-fix: scan all Go modules including tools/go.mod - #76
Conversation
WalkthroughThe CVE-fix workflow now scans all eligible Go modules, aggregates module verdicts, reports per-module results, and documents nested-module handling. Tests cover version comparison, module discovery, manifest checks, and verdict aggregation. The skill version is incremented. ChangesMulti-module Go scanning
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
I didn't have time to properly test this PR thoroughly. The point of is PR is to acknowledge that for Go projects more than one "go.mod" file may exists, and they should all be checked to ensure that the project has a given vulnerability fixed. For example, for https://redhat.atlassian.net/browse/EDM-4164 the scan would indicate that the vulnerability is not present, which is incorrect given that "toold/go.mod" version was unpatched. See https://github.com/flightctl/flightctl-ui/security/dependabot/146 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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`:
- Line 645: Update the repository scanning condition to remove the literal
build_location == "." check and rely on the resolved work_dir and repo_dir
comparison, preserving the Go-specific behavior while allowing equivalent paths
such as "./" to be aggregated.
- Around line 333-342: Update the scan flow around scan_go and
scan_go_tool_module to detect Go source files recursively, rather than relying
on mod_dir.glob("*.go"), so modules containing files under cmd, pkg, or other
subdirectories run govulncheck first. Retain the tool-only fallback only when
govulncheck fails for a qualifying tool-only module, and add a regression module
containing only cmd/app/main.go.
- Around line 180-182: Update the go.mod filtering loop to evaluate
GO_MOD_SKIP_DIRS only against each module path relative to repo_dir, excluding
repository ancestors while preserving skips for matching directories beneath the
scan root. Keep the resulting path handling portable and repository-relative.
- Around line 367-383: Update the top-level result construction around
aggregate_verdict and the returned scan_tool/scan_exit_code fields so metadata
reflects the module producing the overall verdict, rather than always using
primary. Select the relevant module result from module_results based on
overall_verdict, while preserving the existing combined output and verdict
aggregation behavior.
- Around line 260-275: Update _parse_semver and compare_go_versions to parse and
compare the complete Go version string using SemVer-aware rules, including
prerelease, build metadata, and trailing content; do not collapse prereleases or
unsupported forms to the release tuple. Ensure prereleases below the fixed
release remain vulnerable, reject invalid formats safely, and add coverage for
prerelease and build-metadata cases.
In `@cve-fix/skills/scan.md`:
- Around line 40-44: Update the module-discovery guidance to exclude manifests
under .git and vendor: in cve-fix/skills/scan.md lines 40-44, state that
scanning includes every go.mod outside those directories; apply the same
exclusions when listing discovered modules in cve-fix/skills/start.md lines
109-111.
- Around line 97-98: Update the Step 3 VEX closure guidance in the
modules_scanned rule to explicitly state that informational results count as
unaffected. Preserve the existing requirement that every module be patched or
unaffected, while clarifying that informational modules may be closed when their
vulnerable symbols are unreachable.
- Around line 32-37: Update the scan command around FIXED_VERSION so the
variable is optional: use the known fixed version from context.md when
available, but omit the assignment or provide an empty value when it is unknown
instead of expanding the {fixed_version} placeholder. Keep the existing
OUTPUT_DIR and scripts/scan.py arguments unchanged.
- Around line 32-34: Update the scanner command in scan.md to invoke the
existing sibling-relative ../scripts/scan.py path instead of scripts/scan.py,
while preserving all environment variables and arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 87d14bb5-03d3-4c19-9d3e-71851cafc035
📒 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 (7)
**/{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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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: Keep each workflow'sSKILL.mdthin and under 30 lines, with only the entry-point frontmatter and minimal orchestration details.
When modifying a workflow'sSKILL.md, bump that workflow's version in the YAML frontmatter according to the scope of the behavioral change (PATCH/MINOR/MAJOR).
SKILL.mdshould referenceguidelines.mdand may referenceskills/controller.md, using relative paths only.
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
**/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
**/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.
Workflow behavior should be implemented in
skills/*.mdphase files rather than inSKILL.md, keepingSKILL.mdas the thin entry point.
Files:
cve-fix/skills/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.md
🧠 Learnings (6)
📚 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/start.mdcve-fix/skills/scan.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-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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.md
🪛 LanguageTool
cve-fix/skills/scan.md
[typographical] ~44-~44: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...the repository, not just the root module. The JSON output includes a `modules_sca...
(WRB_QUESTION_MARK)
[style] ~51-~51: Consider using the typographical ellipsis character here instead.
Context: ...o.mod| Application/runtime binaries |govulncheck ./...| |tools/go.mod | Dev tools (mockg...
(ELLIPSIS)
🪛 Ruff (0.15.21)
cve-fix/scripts/scan.py
[warning] 239-239: Unused function argument: cve_id
(ARG001)
🔇 Additional comments (4)
cve-fix/SKILL.md (1)
3-3: LGTM!cve-fix/skills/scan.md (1)
45-57: LGTM!Also applies to: 79-81, 144-149
cve-fix/skills/start.md (1)
89-89: LGTM!cve-fix/scripts/scan.py (1)
42-51: LGTM!Also applies to: 278-307, 391-399, 536-536, 587-593, 701-702
496a249 to
8ed9031
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 644-662: Update the Go repository-root branch around
scan_go_repository and the computed base_images so the aggregated verdict
applies the same base-image fallback as the per-directory path before assess_vex
runs. Ensure packages found only in base_images produce the in_base_image
verdict, while preserving existing scan and manifest verdicts when applicable.
In `@cve-fix/scripts/test_scan.py`:
- Around line 44-65: Extend test_find_and_check_all_go_manifests to create
go.mod files containing the target dependency under .git and vendor directories,
creating those directories as needed. Assert find_go_module_dirs excludes both
ignored locations and check_all_go_manifests returns only the root and tools
manifests with the existing ordering.
In `@cve-fix/SKILL.md`:
- Line 3: Update the workflow version in SKILL.md from 0.3.2 to 0.4.0, using a
minor version bump consistent with the changed workflow steps and scanning
rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d7e1569d-9d21-4692-8764-8b28996e2579
📒 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 (7)
**/{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/skills/start.mdcve-fix/skills/scan.mdcve-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).
**/*.md: Workflow content must use plain Markdown and must not contain IDE-specific syntax.
All file references in workflow Markdown must be relative to the referencing file's location.
In attended mode, workflows must not auto-advance; they must wait for user input between phases unless explicit unattended mode is documented.
All significant workflow outputs must be persisted under.artifacts/{workflow-name}/{context}/.
Before applying documentation changes to repository files, run Vale validation.
Before destructive Git operations, verify the repository state withgit status.
Before creating a PR or MR, confirm the branch and base before pushing.
Only cve-fix/close, design/sync, and sizing/applymay write to Jira, and each requires explicit approval.
Files:
cve-fix/skills/start.mdcve-fix/skills/scan.mdcve-fix/SKILL.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/skills/start.mdcve-fix/skills/scan.mdcve-fix/SKILL.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.
Controllers must reference sibling skills as
phase-name.md, notskills/phase-name.md.
Files:
cve-fix/skills/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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
**/{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
**/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 directory must contain aSKILL.mdentry point with YAML frontmatter containingname,version, anddescription.
SKILL.mdmust remain under 30 lines; detailed behavior belongs inguidelines.mdandskills/.
Any directory containingSKILL.mdis automatically discovered by the installer.
When behavioral workflow files change, update the workflow version inSKILL.mdaccording to semver: patch for wording or formatting changes, minor for changed steps/rules/templates/phases, and major for removed or renamed phases or commands.
The version bump must be included in the same commit as the behavioral change; do not create a separate version-bump commit.
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
🧠 Learnings (7)
📚 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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.md
📚 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/skills/start.mdcve-fix/skills/scan.mdcve-fix/SKILL.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/start.mdcve-fix/skills/scan.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/skills/start.mdcve-fix/skills/scan.mdcve-fix/SKILL.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
🪛 LanguageTool
cve-fix/skills/scan.md
[typographical] ~44-~44: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...the repository, not just the root module. The JSON output includes a `modules_sca...
(WRB_QUESTION_MARK)
[style] ~51-~51: 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.0)
cve-fix/scripts/scan.py
[warning] 239-239: Unused function argument: cve_id
(ARG001)
🔇 Additional comments (16)
cve-fix/scripts/test_scan.py (1)
1-42: LGTM!Also applies to: 68-69
cve-fix/scripts/scan.py (7)
180-182: Skip-dir check still matches repository ancestors.
gomod.partsincludes the path aboverepo_dir, so a checkout under a directory namedvendorornode_modulesdiscovers zero modules. Compare only the repo-relative parts.
260-275:_parse_semverstill truncates prerelease/build metadata.
v0.52.0-rc.1compares equal tov0.52.0, yieldingpatched→absentand a false VEX justification.
333-342: Tool-only detection viamod_dir.glob("*.go")still misses subdirectory packages.Modules whose sources live only under
cmd/orpkg/bypassgovulncheckentirely and get a version-only verdict without reachability analysis.
367-383: Top-levelscan_tool/scan_exit_codestill taken from the root module only.The aggregated verdict can come from a nested module while metadata describes the root, producing contradictory output (e.g.
verdict=scan_failedwith exit code 0).
645-645: Repository aggregation still gated on the literal".".
"./"resolves to the repo root but takes the single-module path, so vulnerable secondary modules are missed. The resolved-path comparison alone suffices.
42-52: LGTM!
587-593: LGTM!Also applies to: 701-702
cve-fix/skills/scan.md (5)
34-34: Use the sibling-relative scanner path in the command block.The prose references
../scripts/scan.py, but the executable example invokesscripts/scan.py. From this skill directory, use../scripts/scan.pyconsistently.Source: Path instructions
32-37: MakeFIXED_VERSIONoptional.When no fixed version is known,
FIXED_VERSION={fixed_version}passes a literal placeholder to the scanner. Omit the assignment or expand it to an empty value when unavailable.
40-45: Document the module-discovery exclusions.The scanner does not inspect every
go.mod; manifests under.gitandvendorare excluded. State those exclusions so agents do not manually scan irrelevant modules.Source: Path instructions
97-98: Explicitly classifyinformationalas unaffected.The VEX rule should state that
informationalmodules count as unaffected; otherwise agents may incorrectly block closure for unreachable vulnerable symbols.
79-81: LGTM!Also applies to: 144-149
cve-fix/skills/start.md (2)
109-112: Exclude.gitandvendorwhen listing Go modules.The scanner skips manifests below these directories, but this instruction says to list every
go.mod. Add the same exclusions used by the scanner.Source: Path instructions
89-89: LGTM!cve-fix/SKILL.md (1)
13-13: LGTM!
| --- | ||
| name: cve-fix | ||
| version: 0.3.1 | ||
| version: 0.3.2 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Bump the workflow version to 0.4.0.
This PR changes workflow steps and scanning rules, so the patch bump to 0.3.2 is insufficient. Use a minor bump for the behavioral change, in the same commit.
As per coding guidelines, changed workflow steps/rules require a minor version bump.
🤖 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/SKILL.md` at line 3, Update the workflow version in SKILL.md from
0.3.2 to 0.4.0, using a minor version bump consistent with the changed workflow
steps and scanning rules.
Source: Coding guidelines
adalton
left a comment
adalton
left a comment
There was a problem hiding this comment.
Thanks for the contribution — the motivation is clear and the PR description is well-structured. Multi-module Go scanning addresses a real gap.
There are several issues to address before merge, ranging from bugs that could produce incorrect scan verdicts to project convention violations. I've left inline comments with details on each.
Must-fix summary:
- Version bump should be MINOR (0.4.0), not PATCH (0.3.2) — per AGENTS.md versioning rules
find_go_module_dirsskip-directory check operates on absolute path parts, not relativehas_go_sourcesonly checks the module root dir, missing standard Go layouts (cmd/,pkg/, etc.)- Top-level
scan_tool/scan_exit_codealways come from the root module, even when the verdict comes from a different one - Base-image classification (
in_base_image) is silently dropped for the aggregated Go path build_location == "."string guard is fragile —"./"would bypass multi-module scanning
Should-fix:
7. Script path in scan.md code block doesn't match the prose (scripts/scan.py vs ../scripts/scan.py)
8. FIXED_VERSION shown unconditionally in the code block — should be marked optional
9. Docs say "every go.mod" without mentioning .git/vendor exclusions
10. _parse_semver silently accepts pre-release suffixes (e.g. v0.52.0-rc.1 parses as v0.52.0)
11. Tests don't cover .git/vendor exclusion in find_go_module_dirs
Also note: the PR description says "0.3.0 → 0.4.0" but the actual diff shows 0.3.1 → 0.3.2 — the description doesn't match the code.
8ed9031 to
2690423
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cve-fix/scripts/scan.py (2)
663-680: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument
FIXED_VERSIONin the usage text.Line 705 reads a new environment variable,
FIXED_VERSION. TheEnvironment:block lists onlySCAN_TIMEOUTandOUTPUT_DIR. An agent that reads--helpcannot discover the variable that controls semantic-version comparison.🩹 Proposed fix
"Environment:\n" " SCAN_TIMEOUT Seconds before scan times out (default: 300)\n" + " FIXED_VERSION Version that fixes the CVE (e.g., v0.52.0).\n" + " Used for Go tool-only modules that govulncheck\n" + " cannot analyze.\n" " OUTPUT_DIR Directory for JSON output (default: cwd)",As per path instructions, script conventions must be documented in the docstring so skill files can invoke them correctly.
🤖 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 663 - 680, Update the usage text in the argument-help block of the scan entry point to document the FIXED_VERSION environment variable alongside SCAN_TIMEOUT and OUTPUT_DIR, including its role in semantic-version comparison. Keep the existing help formatting and descriptions unchanged.Source: Path instructions
630-655: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the multi-module Go output format.
Update the
check-manifestdocstring, usage text, andcve-fix/skills/report.mdto state that Go repository roots can print multiple path-prefixed matches joined by;. The report skill uses only the exit status, so no version parser requires changes.🤖 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 630 - 655, Update check_manifest_command’s docstring and usage text to document that Go repository roots may output multiple path-prefixed manifest matches joined by “; ”. Add the same output-format clarification to the check-manifest guidance in report.md, while leaving exit-status handling and version parsing unchanged.Source: Path instructions
🤖 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 243-256: Update scan_go_tool_module to receive and use the
repository-relative module directory when building the “Module” scan output,
rather than the resolved absolute mod_dir. Adjust its callers to pass the
relative path while retaining absolute paths for filesystem operations, ensuring
scan-result.json contains only relative file references.
- Around line 394-404: The module loop around scan_go currently performs
sequential scans, allowing total runtime to grow by SCAN_TIMEOUT for every
module. Update this flow to enforce a single total scan budget across all module
scans, propagating the remaining time to each invocation; alternatively,
document the resulting worst-case duration in scan.md if preserving
per-invocation timeouts. Keep the existing fallback through scan_go_tool_module
for tool-only failures.
- Around line 54-60: Reorder the nested Go module check in detect_language so
package.json and Python manifest checks run first, preserving their
classifications for Node and Python repositories. Only probe
find_go_module_dirs(work_dir) after those cheaper root-manifest checks and
before the final fallback, without changing the existing Go detection behavior.
- Around line 199-212: Update check_single_go_manifest to inspect only require
entries and match the requested package as a complete module-path token, not an
arbitrary substring; exclude the repository’s module declaration and prevent
prefix collisions such as crypto versus cryptobyte. Preserve returning the
stripped matching require line and returning an empty string when no valid match
exists.
- Around line 703-723: Update the documentation for the scan output near the
repository-root Go scan flow to state that FIXED_VERSION applies only to
repository-root Go scans. Document that modules_scanned is omitted when no
modules are scanned, rather than claiming the JSON output always contains it.
- Around line 443-453: Update the return dictionary in the Go scan result flow
to populate toolchain_matched from the representative module at primary_idx,
matching the existing scan_tool, scan_exit_code, and target_go_version metadata
instead of hardcoding None. Reuse the value computed by scan_go for that module
and preserve the remaining result fields unchanged.
In `@cve-fix/scripts/test_scan.py`:
- Around line 56-75: Extend the tests for tool_module_verdict to assert absent
when no manifest line exists, present_by_version when FIXED_VERSION is unset,
and scan_failed when resolution fails, using the existing test helpers and
inputs. Extend aggregate_verdict coverage to assert scan_failed for an empty
verdict list while preserving the current precedence assertions.
In `@cve-fix/skills/scan.md`:
- Around line 106-108: Update the VEX closure rule near the modules_scanned
guidance and the related verdict table to use only the scanner’s exact verdict
names: absent, informational, present_by_version, in_base_image, and
scan_failed. Explicitly identify which of these verdicts permit closure and
which block it, and apply the same vocabulary consistently across the workflow
and schema consumers.
- Around line 32-46: Update the scanner instructions around the scan.py command
to state that it must run exactly once, using either the command without
FIXED_VERSION when no fixed version is known or the command with FIXED_VERSION
when context.md provides one; label these two forms as mutually exclusive
alternatives and require the fixed-version form for secondary module
comparisons.
---
Outside diff comments:
In `@cve-fix/scripts/scan.py`:
- Around line 663-680: Update the usage text in the argument-help block of the
scan entry point to document the FIXED_VERSION environment variable alongside
SCAN_TIMEOUT and OUTPUT_DIR, including its role in semantic-version comparison.
Keep the existing help formatting and descriptions unchanged.
- Around line 630-655: Update check_manifest_command’s docstring and usage text
to document that Go repository roots may output multiple path-prefixed manifest
matches joined by “; ”. Add the same output-format clarification to the
check-manifest guidance in report.md, while leaving exit-status handling and
version parsing unchanged.
🪄 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: 54c225ab-5825-44a9-9ae6-f33677d403e1
📒 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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.md
**/*.{md,py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Workflow content must use plain markdown and contain no IDE-specific syntax.
Files:
cve-fix/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.py
**/{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/start.mdcve-fix/skills/scan.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Save all significant workflow outputs under
.artifacts/{workflow-name}/{context}/.
Files:
cve-fix/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.py
**/{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/start.mdcve-fix/skills/scan.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
**/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
**/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-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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.md
📚 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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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/start.mdcve-fix/skills/scan.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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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/skills/start.mdcve-fix/SKILL.mdcve-fix/skills/scan.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
🪛 LanguageTool
cve-fix/skills/scan.md
[typographical] ~53-~53: 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] ~60-~60: Consider using the typographical ellipsis character here instead.
Context: ...o.mod| Application/runtime binaries |govulncheck ./...| |tools/go.mod | Dev tools (mockg...
(ELLIPSIS)
🔇 Additional comments (15)
cve-fix/scripts/test_scan.py (5)
1-15: LGTM!
18-54: LGTM!
77-86: LGTM!
88-121: LGTM!
123-149: LGTM!cve-fix/scripts/scan.py (5)
179-196: LGTM!
264-341: LGTM!
344-373: LGTM!
456-464: LGTM!
601-601: LGTM!cve-fix/SKILL.md (2)
13-13: LGTM!
3-3: 📐 Maintainability & Code QualityKeep the review comment resolved.
cve-fix/SKILL.mdmeets the required metadata, 25-line limit, references, and command mappings.cve-fix/skills/scan.md (2)
56-67: LGTM!Also applies to: 88-90, 154-159
48-54: 🗄️ Data Integrity & IntegrationKeep the all-module scanning contract consistent.
The two skill files document different conditions for complete Go module coverage. Confirm the implementation behavior, then apply one rule in both files.
cve-fix/skills/scan.md#L48-L54: document whether all-module scanning requiresbuild_locationto be..cve-fix/skills/start.md#L109-L113: apply the same condition when stating that/scanchecks every discovered module.Source: Path instructions
cve-fix/skills/start.md (1)
89-89: LGTM!
| def detect_language(work_dir: Path) -> str: | ||
| """Detect project language from manifest files.""" | ||
| if (work_dir / "go.mod").is_file(): | ||
| return "go" | ||
| # Nested-only layouts (e.g. tools/go.mod with no root module) | ||
| if find_go_module_dirs(work_dir): | ||
| return "go" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Nested Go detection now outranks Node and Python detection, and it walks the whole tree.
Line 59 runs before the package.json and Python manifest checks. A repository that has package.json at the root and any nested go.mod (for example a small Go helper under tools/) is now classified as go. That changes the scanner selection for existing repositories.
find_go_module_dirs also calls rglob("go.mod"), which walks the complete tree on every detect_language call, including calls for Node and Python repositories where the result is discarded.
Move the nested-module probe after the cheaper root-manifest checks.
♻️ Proposed reordering
def detect_language(work_dir: Path) -> str:
"""Detect project language from manifest files."""
if (work_dir / "go.mod").is_file():
return "go"
- # Nested-only layouts (e.g. tools/go.mod with no root module)
- if find_go_module_dirs(work_dir):
- return "go"
if (work_dir / "package.json").is_file():
return "node"
for manifest in ("requirements.txt", "pyproject.toml", "setup.py"):
if (work_dir / manifest).is_file():
return "python"
+ # Nested-only layouts (e.g. tools/go.mod with no root module)
+ if find_go_module_dirs(work_dir):
+ return "go"
return "unknown"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cve-fix/scripts/scan.py` around lines 54 - 60, Reorder the nested Go module
check in detect_language so package.json and Python manifest checks run first,
preserving their classifications for Node and Python repositories. Only probe
find_go_module_dirs(work_dir) after those cheaper root-manifest checks and
before the final fallback, without changing the existing Go detection behavior.
There was a problem hiding this comment.
Ultimately the single language detection for the same project won't work well for all cases.
It depends on the CVE which language is impacted and which modules the scanner should review.
I created a follow-up PR based off this one that addresses this issue.
#101
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh pr view 101 \
--repo flightctl/ai-workflows \
--json number,title,state,body,url,files \
--jq '{number, title, state, url, body, files: [.files[] | {path, additions, deletions}]}'Length of output: 5825
@celdrake, agreed. I verified that #101 addresses the broader issue.
The follow-up records the CVE-specific primary ecosystem in context.md and uses LANGUAGE for scanner selection. This prevents a single repository-wide language classification from selecting the wrong scanner in polyglot repositories.
The proposed detect_language() reordering is therefore not sufficient as the long-term solution. I will not pursue that narrow change on this PR.
🧬 Code Graph Analysis Results
cve-fix/scripts/_common.py
Lines 28–43 — run
Runs scanner commands with merged environment variables, optional working directory, timeout handling, and returns (exit_code, combined_output). Returns exit code 127 for missing commands and 124 for timeouts.
Lines 46–54 — is_successful_scan
Defines scanner-specific successful exit codes: govulncheck uses 0 and 3; npm audit and pip-audit use 0 and 1.
Lines 57–68 — validate_work_dir
Resolves build_location, verifies it remains within repo_dir, and returns the resolved path or None when it escapes the repository.
Lines 71–75 — write_json
Creates the output directory, writes the JSON result to disk, and prints it to stdout.
Lines 78–79 — timestamp
Returns the current UTC timestamp in YYYY-MM-DDTHH:MM:SSZ format.
cve-fix/scripts/verify.py
Lines 193–263 — main
Accepts an explicitly supplied language and optional Go toolchain version, validates the CVE and build location, selects the corresponding verifier, classifies the result, and writes verify-result.json. Unsupported languages produce a failed scan result.
cve-fix/scripts/check_existing_prs.py
Lines 121–163 — main
Parses repository, branch, CVE, and package arguments; searches for existing PRs by CVE, bot update, and package; emits JSON describing the first match or an error.
✏️ Learnings added
Learnt from: celdrake
Repo: flightctl/ai-workflows PR: 76
File: cve-fix/scripts/scan.py:54-60
Timestamp: 2026-08-10T12:09:09.329Z
Learning: For the `cve-fix` workflow, polyglot repositories must select the scanner from the CVE-specific primary ecosystem, not from a single repository-wide language detection result. The workflow records the primary ecosystem in `context.md` and passes it as `LANGUAGE` to the scan step; other detected ecosystems can be additional and out of scope for that CVE.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Made-with: Cursor
2690423 to
6da1abd
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/test_scan.py`:
- Around line 114-123: Resolve the temporary-directory base path once in each
affected test, including test_detect_language_nested_go_only,
test_find_and_check_all_go_manifests, and
test_find_go_module_dirs_skips_nested_vendor_not_ancestor, before constructing
expected module paths. Use that resolved base for repo and child path creation
so comparisons with find_go_module_dirs, which returns resolved paths, remain
consistent across platforms.
In `@cve-fix/skills/report.md`:
- Line 300: Update the match-formatting text so the semicolon remains in the
Markdown code span while the following space is outside it, resolving the MD038
violation without changing the output format.
In `@cve-fix/skills/scan.md`:
- Around line 53-57: Update the inline Go fallback documented in the scan
instructions to discover and scan each eligible go.mod module, not just the
current module. Preserve the .git, vendor, and node_modules exclusions, then
aggregate all module scan results before applying the final vulnerability
verdict.
- Around line 58-60: Update the `modules_scanned` documentation in `scan.md` to
state that it is emitted as an empty array when no modules are scanned, matching
the behavior of `scan.py`; retain the existing description for non-empty results
and ensure the documented field type remains consistent with the producer.
🪄 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: b22cd713-28dc-4a97-b405-dc0da574885d
📒 Files selected for processing (4)
cve-fix/scripts/scan.pycve-fix/scripts/test_scan.pycve-fix/skills/report.mdcve-fix/skills/scan.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/{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/skills/report.mdcve-fix/skills/scan.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/skills/report.mdcve-fix/skills/scan.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/skills/report.mdcve-fix/skills/scan.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/report.mdcve-fix/skills/scan.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/report.mdcve-fix/skills/scan.md
**/*.{md,py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Workflow content must use plain markdown and contain no IDE-specific syntax.
Files:
cve-fix/skills/report.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.py
**/{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/report.mdcve-fix/skills/scan.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Save all significant workflow outputs under
.artifacts/{workflow-name}/{context}/.
Files:
cve-fix/skills/report.mdcve-fix/skills/scan.mdcve-fix/scripts/test_scan.pycve-fix/scripts/scan.py
**/{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/report.mdcve-fix/skills/scan.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-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/report.mdcve-fix/skills/scan.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/report.mdcve-fix/skills/scan.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/report.mdcve-fix/skills/scan.md
📚 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/skills/report.mdcve-fix/skills/scan.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/report.mdcve-fix/skills/scan.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/skills/report.mdcve-fix/skills/scan.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/skills/report.mdcve-fix/skills/scan.md
📚 Learning: 2026-08-10T12:09:09.329Z
Learnt from: celdrake
Repo: flightctl/ai-workflows PR: 76
File: cve-fix/scripts/scan.py:54-60
Timestamp: 2026-08-10T12:09:09.329Z
Learning: In the cve-fix workflow, select the scanner based on the CVE-specific primary ecosystem recorded in context.md, and pass that ecosystem as LANGUAGE to the scan step. Do not rely on a single repository-wide language detection result; other detected ecosystems may be present but are out of scope for the current CVE.
Applied to files:
cve-fix/scripts/test_scan.pycve-fix/scripts/scan.py
🪛 GitHub Actions: Lint / 3_Markdown Lint.txt
cve-fix/skills/report.md
[error] 300-300: markdownlint MD038/no-space-in-code: Spaces inside code span elements near "; ". Remove the spaces inside the code span.
🪛 GitHub Actions: Lint / Markdown Lint
cve-fix/skills/report.md
[error] 300-300: markdownlint MD038/no-space-in-code: Spaces inside code span elements. Remove the spaces inside the code span near column 65 (context: "; ").
🪛 GitHub Check: Markdown Lint
cve-fix/skills/report.md
[failure] 300-300: Spaces inside code span elements
cve-fix/skills/report.md:300:65 MD038/no-space-in-code Spaces inside code span elements [Context: "; "] https://github.com/DavidAnson/markdownlint/blob/v0.37.4/doc/md038.md
🪛 LanguageTool
cve-fix/skills/report.md
[style] ~301-~301: Consider using the typographical ellipsis character here instead.
Context: ...h>: joined by; (for examplego.mod: require ...; tools/go.mod: ...`). If the script isn...
(ELLIPSIS)
cve-fix/skills/scan.md
[typographical] ~58-~58: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...der .git, vendor, and node_modules. When modules are scanned, the JSON outp...
(WRB_QUESTION_MARK)
[style] ~69-~69: Consider using the typographical ellipsis character here instead.
Context: ...o.mod| Application/runtime binaries |govulncheck ./...| |tools/go.mod | Dev tools (mockg...
(ELLIPSIS)
🔇 Additional comments (3)
cve-fix/scripts/scan.py (1)
12-21: LGTM!Also applies to: 48-66, 185-257, 260-297, 300-410, 412-501, 638-638, 667-699, 749-809
cve-fix/scripts/test_scan.py (1)
1-16: LGTM!Also applies to: 20-112, 125-158, 160-182, 185-186
cve-fix/skills/scan.md (1)
29-51: LGTM!Also applies to: 62-75, 97-99, 115-118, 164-169
| def test_detect_language_nested_go_only(self) -> None: | ||
| with tempfile.TemporaryDirectory() as tmp: | ||
| repo = Path(tmp) | ||
| tools = repo / "tools" | ||
| tools.mkdir() | ||
| (tools / "go.mod").write_text( | ||
| "module example.com/tools\n\ngo 1.25.0\n" | ||
| ) | ||
| self.assertEqual(detect_language(repo), "go") | ||
| self.assertEqual(find_go_module_dirs(repo), [tools]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Resolve the temporary directory before you compare module paths.
find_go_module_dirs resolves repo_dir and returns resolved module paths. tempfile.TemporaryDirectory() can return a symlinked path, for example /var/folders/... on macOS, which resolves to /private/var/folders/.... The expected paths here are built from the unresolved tmp, so assertEqual(find_go_module_dirs(repo), [tools]) fails on such platforms. The same risk exists at Line 148 and Lines 181-182.
Resolve the base path once in each test.
🩹 Proposed fix
def test_detect_language_nested_go_only(self) -> None:
with tempfile.TemporaryDirectory() as tmp:
- repo = Path(tmp)
+ repo = Path(tmp).resolve()
tools = repo / "tools"Apply the same change in test_find_and_check_all_go_manifests and test_find_go_module_dirs_skips_nested_vendor_not_ancestor:
- repo = Path(tmp)
+ repo = Path(tmp).resolve()- vendor_parent = Path(tmp) / "vendor" / "myproject"
+ vendor_parent = Path(tmp).resolve() / "vendor" / "myproject"📝 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 test_detect_language_nested_go_only(self) -> None: | |
| with tempfile.TemporaryDirectory() as tmp: | |
| repo = Path(tmp) | |
| tools = repo / "tools" | |
| tools.mkdir() | |
| (tools / "go.mod").write_text( | |
| "module example.com/tools\n\ngo 1.25.0\n" | |
| ) | |
| self.assertEqual(detect_language(repo), "go") | |
| self.assertEqual(find_go_module_dirs(repo), [tools]) | |
| def test_detect_language_nested_go_only(self) -> None: | |
| with tempfile.TemporaryDirectory() as tmp: | |
| repo = Path(tmp).resolve() | |
| tools = repo / "tools" | |
| tools.mkdir() | |
| (tools / "go.mod").write_text( | |
| "module example.com/tools\n\ngo 1.25.0\n" | |
| ) | |
| self.assertEqual(detect_language(repo), "go") | |
| self.assertEqual(find_go_module_dirs(repo), [tools]) |
🤖 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/test_scan.py` around lines 114 - 123, Resolve the
temporary-directory base path once in each affected test, including
test_detect_language_nested_go_only, test_find_and_check_all_go_manifests, and
test_find_go_module_dirs_skips_nested_vendor_not_ancestor, before constructing
expected module paths. Use that resolved base for repo and child path creation
so comparisons with find_go_module_dirs, which returns resolved paths, remain
consistent across platforms.
| **Go repositories with multiple `go.mod` files:** Many Go projects keep a | ||
| root `go.mod` for shipped binaries and additional modules for dev tooling | ||
| (for example `tools/go.mod`, `test/scripts/go.mod`). When `build_location` | ||
| is `.` (the default), `scan.py` scans **every** `go.mod` in the repository, | ||
| not just the root module — excluding paths under `.git`, `vendor`, and |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve multi-module coverage in the manual fallback.
When scan.py is unavailable, the inline Go fallback at Line 77-83 still documents one govulncheck ... ./... run. That command checks only the current module. A repository with tools/go.mod can therefore miss a vulnerable secondary module and produce a false clean result.
Document one scan per eligible module, with the same .git, vendor, and node_modules exclusions, and aggregate the results before applying the verdict. This is required by the PR objective to detect vulnerabilities in secondary 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/skills/scan.md` around lines 53 - 57, Update the inline Go fallback
documented in the scan instructions to discover and scan each eligible go.mod
module, not just the current module. Preserve the .git, vendor, and node_modules
exclusions, then aggregate all module scan results before applying the final
vulnerability verdict.
| `node_modules`. When modules are scanned, the JSON output includes a | ||
| `modules_scanned` array with per-module verdicts and resolved versions; | ||
| the field is omitted when no modules were scanned. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align modules_scanned documentation with the emitted JSON.
cve-fix/scripts/scan.py returns "modules_scanned": [] when no go.mod files are found. This text says the field is omitted. Consumers can treat a missing field and an empty array differently. Document the empty-array case, or change the producer and tests together.
Suggested correction
- the field is omitted when no modules were scanned.
+ when no Go modules are found, `modules_scanned` is present as an empty array.As per path instructions, schema field names and types must match between producer and consumer files.
📝 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.
| `node_modules`. When modules are scanned, the JSON output includes a | |
| `modules_scanned` array with per-module verdicts and resolved versions; | |
| the field is omitted when no modules were scanned. | |
| `node_modules`. When modules are scanned, the JSON output includes a | |
| `modules_scanned` array with per-module verdicts and resolved versions; | |
| when no Go modules are found, `modules_scanned` is present as an empty array. |
🧰 Tools
🪛 LanguageTool
[typographical] ~58-~58: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...der .git, vendor, and node_modules. When modules are scanned, the JSON outp...
(WRB_QUESTION_MARK)
🤖 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 58 - 60, Update the `modules_scanned`
documentation in `scan.md` to state that it is emitted as an empty array when no
modules are scanned, matching the behavior of `scan.py`; retain the existing
description for non-empty results and ensure the documented field type remains
consistent with the producer.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cve-fix/skills/report.md`:
- Around line 302-303: Update the inline fallback in the report instructions to
enumerate every go.mod under <repo_dir>, excluding .git and vendor, including
nested modules such as tools/go.mod. Require an exact, case-sensitive match of
the package’s module path in require directives rather than a case-insensitive
substring grep, and report the result as inconclusive if this check cannot run.
🪄 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: e61839d5-8daf-4efb-947f-a2a040ba88fe
📒 Files selected for processing (1)
cve-fix/skills/report.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/{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/skills/report.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/skills/report.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/skills/report.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/report.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/report.md
**/*.{md,py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Workflow content must use plain markdown and contain no IDE-specific syntax.
Files:
cve-fix/skills/report.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/report.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Save all significant workflow outputs under
.artifacts/{workflow-name}/{context}/.
Files:
cve-fix/skills/report.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/report.md
🧠 Learnings (7)
📚 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/report.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/report.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/report.md
📚 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/skills/report.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/report.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/skills/report.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/skills/report.md
🪛 LanguageTool
cve-fix/skills/report.md
[style] ~301-~301: Consider using the typographical ellipsis character here instead.
Context: ... joined by semicolon-space (for example go.mod: require ...; tools/go.mod: ...). If the script isn...
(ELLIPSIS)
| If the script isn't available, run the equivalent inline: grep the relevant | ||
| manifest file(s) for the package name, case-insensitively. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the inline Go fallback preserve the exact module check.
The fallback uses a case-insensitive substring grep. The preceding instruction requires a full, case-sensitive module-token match across every eligible go.mod, including nested modules. This fallback can match the wrong module or miss a secondary module such as tools/go.mod.
Require enumeration of all go.mod files under <repo_dir> except .git and vendor, exact matching of the module path in require directives, and an inconclusive result when that check cannot run.
Suggested correction
-If the script isn't available, run the equivalent inline: grep the relevant
-manifest file(s) for the package name, case-insensitively.
+If the script isn't available, enumerate every `go.mod` under `<repo_dir>`
+except `.git` and `vendor`. For Go, match the package as a full,
+case-sensitive module token in `require` directives. Preserve the
+`<manifest_path>: <line>` output format. If this exact check cannot run,
+treat the result as inconclusive rather than excluding the ticket.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If the script isn't available, run the equivalent inline: grep the relevant | |
| manifest file(s) for the package name, case-insensitively. | |
| If the script isn't available, enumerate every `go.mod` under `<repo_dir>` | |
| except `.git` and `vendor`. For Go, match the package as a full, | |
| case-sensitive module token in `require` directives. Preserve the | |
| `<manifest_path>: <line>` output format. If this exact check cannot run, | |
| treat the result as inconclusive rather than excluding the ticket. |
🤖 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/report.md` around lines 302 - 303, Update the inline fallback
in the report instructions to enumerate every go.mod under <repo_dir>, excluding
.git and vendor, including nested modules such as tools/go.mod. Require an
exact, case-sensitive match of the package’s module path in require directives
rather than a case-insensitive substring grep, and report the result as
inconclusive if this check cannot run.
adalton
left a comment
adalton
left a comment
There was a problem hiding this comment.
Good progress since the last round. The prerelease parsing, exact token matching, toolchain propagation, and module_label fix are solid improvements. Two correctness issues remain, plus quality items I'd want addressed for a workflow other teams will run in production.
Must-fix (correctness/safety):
detect_languageordering regresses polyglot repos (line 65)scan_failedpromoted toin_base_imageis unsafe (line 764)
Should-fix (quality):
3. rglob traverses .git/vendor/node_modules before filtering (line 189)
4. check_single_go_manifest matches inside replace/exclude blocks (line 237)
5. Redundant check_all_go_manifests call re-traverses and re-reads (line 466)
6. Per-Module Results section unconditional in scan.md template (line 164)
| return "go" | ||
| # Nested-only layouts (e.g. tools/go.mod with no root module) | ||
| if find_go_module_dirs(work_dir): | ||
| return "go" |
There was a problem hiding this comment.
Must-fix: ordering regresses polyglot repos.
The nested find_go_module_dirs check before package.json changes existing behavior. A Node.js project with package.json at root and tools/go.mod would now detect as "go" instead of "node" — the scan would run govulncheck instead of npm audit.
Move this check after all root-manifest checks, as a last resort before "unknown":
def detect_language(work_dir: Path) -> str:
if (work_dir / "go.mod").is_file():
return "go"
if (work_dir / "package.json").is_file():
return "node"
for manifest in ("requirements.txt", "pyproject.toml", "setup.py"):
if (work_dir / manifest).is_file():
return "python"
if find_go_module_dirs(work_dir):
return "go"
return "unknown"This preserves pre-existing behavior for repos with root manifests while adding nested-only Go detection for repos that have no root manifest.
Also add a test: root package.json + nested tools/go.mod should detect "node", not "go".
| """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")): |
There was a problem hiding this comment.
Should-fix: rglob traverses skipped directories before filtering.
rglob("go.mod") walks the entire tree — including .git, vendor, and node_modules — before filtering results. For flightctl-ui (the motivating repo), node_modules alone can have tens of thousands of entries.
This is also called from detect_language on every scan invocation when there's no root go.mod (after fixing the ordering above), so even Node/Python scans pay this cost.
os.walk with in-place pruning avoids entering skipped trees:
def find_go_module_dirs(repo_dir: Path) -> list[Path]:
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 != repo_dir, str(path)))
return modulesNo post-hoc filtering needed since excluded directories are never entered.
| if len(fields) < 2 or fields[1] == "(": | ||
| continue | ||
| path = fields[1] | ||
| else: |
There was a problem hiding this comment.
Should-fix: matches inside replace/exclude blocks.
The docstring says "require entries only" but the function doesn't track block context. Single-line directives like replace golang.org/x/crypto => ... are correctly skipped (keyword is the first field), but entries inside multi-line blocks are matched:
replace (
golang.org/x/crypto => ./local-fork
)Here golang.org/x/crypto is the first field, not in skip_directives, not "require" — so it falls through to the path match at line 239.
Fix: track whether you're inside a directive block. Only match when inside a require (...) block or on a bare require line.
Also add a test: a go.mod with a package only in replace (...) (not in require) should return "".
| ), | ||
| }) | ||
|
|
||
| all_manifests = check_all_go_manifests(repo_dir, package) |
There was a problem hiding this comment.
Should-fix: redundant traversal and I/O.
check_all_go_manifests calls find_go_module_dirs (re-traverses the tree) and then check_single_go_manifest on each module (re-reads each go.mod). The loop above already collected this data in module_results.
Replace with:
manifest_summary = "; ".join(
f"{m['manifest_path']}: {m['manifest_line']}"
for m in module_results if m['manifest_line']
)| verdict = repo_scan["verdict"] | ||
| modules_scanned = repo_scan["modules_scanned"] | ||
| base_images = check_base_images(work_dir) | ||
| if verdict in ("absent", "scan_failed") and base_images and not manifest_match: |
There was a problem hiding this comment.
Must-fix: scan_failed should not be promoted to in_base_image.
A failed scan means we don't know whether the CVE is present in the application code. Claiming it's only in the base image is an unsafe assumption that could lead to premature VEX closure.
Only absent should be eligible:
if verdict == "absent" and base_images and not manifest_match:
verdict = "in_base_image"|
|
||
| {interpretation of what the verdict means for this specific case} | ||
|
|
||
| ## Per-Module Results (Go multi-module repos) |
There was a problem hiding this comment.
Should-fix: unconditional section.
This section appears in the template for every scan, but modules_scanned only exists for Go multi-module repo-root scans. Node, Python, and Go subdirectory scans would produce an empty table.
Follow the VEX section's pattern:
## Per-Module Results (Go multi-module repos, if applicable)Or add a note: "Include this section only when modules_scanned is present in scan-result.json. Omit entirely for non-Go or subdirectory scans."
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cve-fix/skills/report.md (1)
302-303: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the exact manifest check in the fallback.
The fallback still uses case-insensitive grep. It can miss
tools/go.mod, match a substring instead of the full Go module token, and incorrectly treat an incomplete check as “no match.” Enumerate every eligiblego.modunder<repo_dir>, exclude.git,vendor, andnode_modules, and keep the result inconclusive when the exact check cannot run. This is the same issue reported in the previous review.🤖 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/report.md` around lines 302 - 303, Update the inline fallback in the script-availability guidance to enumerate every eligible go.mod under <repo_dir>, including tools/go.mod, while excluding .git, vendor, and node_modules. Match the package as an exact Go module token rather than a case-insensitive substring, and preserve an inconclusive result whenever the exact manifest check cannot run instead of reporting no match.
🤖 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.
Duplicate comments:
In `@cve-fix/skills/report.md`:
- Around line 302-303: Update the inline fallback in the script-availability
guidance to enumerate every eligible go.mod under <repo_dir>, including
tools/go.mod, while excluding .git, vendor, and node_modules. Match the package
as an exact Go module token rather than a case-insensitive substring, and
preserve an inconclusive result whenever the exact manifest check cannot run
instead of reporting no match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a6271dcb-876b-4539-9d77-f0adef92ce60
📒 Files selected for processing (1)
cve-fix/skills/report.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/{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/skills/report.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/skills/report.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/skills/report.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/report.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/report.md
**/*.{md,py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
Workflow content must use plain markdown and contain no IDE-specific syntax.
Files:
cve-fix/skills/report.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/report.md
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Save all significant workflow outputs under
.artifacts/{workflow-name}/{context}/.
Files:
cve-fix/skills/report.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/report.md
🧠 Learnings (7)
📚 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/report.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/report.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/report.md
📚 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/skills/report.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/report.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/skills/report.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/skills/report.md
🪛 LanguageTool
cve-fix/skills/report.md
[style] ~301-~301: Consider using the typographical ellipsis character here instead.
Context: ... joined by semicolon-space (for example go.mod: require ...; tools/go.mod: ...). If the script isn...
(ELLIPSIS)
2 participants
Summary
Improves the
/scanphase for Go repositories that contain multiplego.modfiles (for examplego.mod+tools/go.mod).This addresses a gap found while fixing flightctl/flightctl#3228 / Dependabot alert #102 for CVE-2026-46595: the root
go.modwas already patched, buttools/go.modstill pinned a vulnerablegolang.org/x/cryptoversion. The previous scan logic only checked the root module, which could lead to a false "absent" verdict and premature VEX closure.Changes
scan.py: Discover everygo.modunder the repository (excluding.git/vendor), scan each module, and aggregate the most severe verdicttools/go.mod, etc.): usego list -mwhengovulncheckcannot analyze them; optionalFIXED_VERSIONenv var enables semver comparisonscan.md: Document multi-module scanning, per-module results table, andFIXED_VERSIONusagestart.md: Note that Go projects may have secondary modules and list them in contexttest_scan.py: Unit tests for manifest discovery, version comparison, and verdict aggregationcve-fix0.3.0 → 0.4.0Example
Expected per-module output in
scan-result.json:.(rootgo.mod)absent(v0.53.0)tools(tools/go.mod)present_by_version(v0.43.0)Overall verdict:
present_by_versionTest plan
python3 cve-fix/scripts/test_scan.py -vmodules_scannedappears inscan-result.jsonoutputMade with Cursor
Workflows affected
cve-fixnow scans all eligible Go modules, including secondary modules such astools/go.mod..git,vendor, andnode_modules.go list -mwhengovulncheckcannot analyze them.FIXED_VERSIONcomparisons classify dependency versions.modules_scanned.Skills, commands, and guidelines
cve-fix/scripts/scan.pynow supports module discovery, per-module scanning, tool-module fallback, semantic-version comparison, and verdict aggregation.cve-fix/skills/scan.mddocuments multi-module scanning, per-module results, manual checks, and all-module VEX closure requirements.cve-fix/skills/start.mdrequires identification and scanning of all eligiblego.modfiles.cve-fix/skills/report.mdrequires exact Go module-token matching during manifest checks and documents multi-module output.cve-fix/scripts/test_scan.pyadds coverage for module discovery, version comparison, verdict classification, aggregation, and nested-module handling.Shared resources and cross-workflow conventions
_shared/resources.