Skip to content

feat: Add vulnerable functions to CLI issue/finding OD-296 - #34

Merged
alerizzo merged 3 commits into
mainfrom
feature/source-id-issues-OD-296
Jul 29, 2026
Merged

feat: Add vulnerable functions to CLI issue/finding OD-296#34
alerizzo merged 3 commits into
mainfrom
feature/source-id-issues-OD-296

Conversation

@pedrobpereira

Copy link
Copy Markdown
Contributor

What

  • Bump the pinned codacy-api schema (fetch-api script) 56.2.957.3.9, picking up CommitIssue.advisoryInformation (OD-284) and SrmItem.advisoryInformation (OD-397).
  • issue, issues, and pull-request --issue show vulnerable/affected functions for SCA issues with a linked OSV advisory (CommitIssue.advisoryInformation): compact one-liner on card views (printIssueCard), full block with advisory ID + published date on detail views (printAdvisoryBlock, wired into printIssueCodeContext).
  • finding and findings get the same treatment from SrmItem.advisoryInformation, closing the gap noted when OD-296 first shipped for issues (SrmItem had no equivalent field server-side until OD-397). finding skips its own block when a linked Codacy issue already renders the same data via printIssueCodeContext, so this is what actually makes vulnerable functions visible for SCA/dependency findings (and any other non-Codacy-source finding), which have no linked issue to borrow it from at all.
  • New summarizeFunctions/printAdvisoryBlock helpers in utils/formatting.ts, shared across all five commands.
  • Included in --output json for all five commands (pickDeep whitelists extended).

Why

OD-296: add affected/vulnerable functions to the findings command. The issues-side plumbing (OD-284's CommitIssue.advisoryInformation) was already usable; the findings-side was blocked on SrmItem not carrying the field server-side, which OD-397 has now shipped.

Testing

  • New tests: 3 in issue.test.ts, 3 in pull-request.test.ts (advisory info on --issue detail), 3 in finding.test.ts, 3 in findings.test.ts — 12 new, 494 total, all passing.
  • tsc --noEmit clean (aside from a pre-existing, unrelated update-notifier module-resolution error in src/utils/update-check.ts).
  • New changeset (minor).

Related

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 24 duplication

Metric Results
Duplication 24

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Surfaces advisoryInformation (advisory ID, vulnerable functions, published
date) across issue, issues, pull-request --issue, finding, and findings:
compact one-liners on list/card views, full blocks on detail views.

finding skips its own block when a linked Codacy issue already renders the
same data via printIssueCodeContext, so SCA/dependency findings (which have
no linked issue) are the case this closes out, now that SrmItem carries
advisoryInformation directly (server-side, API 57.3.9).
Drop the vulnerable-functions/advisoryInformation qualifiers from the
pull-request/issues/issue/findings/finding rows in the command inventory.
@pedrobpereira
pedrobpereira force-pushed the feature/source-id-issues-OD-296 branch from 1903748 to 5838a71 Compare July 28, 2026 16:46
@pedrobpereira
pedrobpereira marked this pull request as ready for review July 28, 2026 16:52

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

The PR successfully updates the CLI to version 57.3.9 of the Codacy API to render vulnerable functions. However, there is a high-risk security flaw: externally sourced advisory strings (names, IDs, and functions) are rendered to the terminal without sanitization, exposing users to potential terminal control character injection (CWE-150). Additionally, while the PR is generally up to standards, a specific acceptance criterion regarding the suppression of redundant advisory blocks in the 'finding' command (when a linked issue is present) appears unaddressed or untested. Finally, the complexity of the printFindingCard function now exceeds project line-count limits.

About this PR

  • The requirement to suppress the advisory block in the 'finding' command when a linked Codacy issue is present appears to be missing or unverified. Ensure logic is implemented to avoid duplicate information in detail views.

Test suggestions

  • Verify compact vulnerable functions line on issues/findings cards
  • Verify truncation logic ('+N more') when more than 3 functions are present
  • Verify full advisory block in detail views (issue/finding/pr --issue)
  • Verify suppression of redundant advisory block in finding detail when linked issue is present
  • Verify advisoryInformation is present in JSON output for all commands
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify suppression of redundant advisory block in finding detail when linked issue is present

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread src/utils/formatting.ts
Comment on lines +280 to +284
export function summarizeFunctions(fns: string[], limit = 3): string {
const shown = fns.slice(0, limit).join(", ");
const more = fns.length > limit ? ` (+${fns.length - limit} more)` : "";
return `${shown}${more}`;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 HIGH RISK

Neutralize terminal control characters in external advisory data to prevent terminal injection attacks.

Suggested change
export function summarizeFunctions(fns: string[], limit = 3): string {
const shown = fns.slice(0, limit).join(", ");
const more = fns.length > limit ? ` (+${fns.length - limit} more)` : "";
return `${shown}${more}`;
}
export function summarizeFunctions(fns: string[], limit = 3): string {
const shown = fns.slice(0, limit).map(sanitizeText).join(", ");
const more = fns.length > limit ? ` (+${fns.length - limit} more)` : "";
return `${shown}${more}`;
}

Comment thread src/commands/findings.ts Outdated
}

// Vulnerable functions (findings with an OSV-linked advisory), compact form
if (item.advisoryInformation) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

The printFindingCard function has exceeded the 50-line limit (currently 53 lines). Additionally, the new vulnerable functions line should only be rendered if there are functions to display to avoid empty labels.

Consider refactoring the status and version metadata construction into a dedicated helper function to reduce complexity, and gate the rendering logic:

Suggested change
if (item.advisoryInformation) {
if (item.advisoryInformation?.vulnerableFunctions?.length) {
console.log(
ansis.dim(`Vulnerable functions: ${summarizeFunctions(item.advisoryInformation.vulnerableFunctions)}`),
);
}

See Issue in Codacy

Comment thread src/utils/formatting.ts
Comment on lines +635 to +645
export function printAdvisoryBlock(advisory: AdvisoryInformation): void {
console.log();
console.log(ansis.bold(`Vulnerable Functions (${advisory.advisoryId})`));
if (advisory.publishedAt) {
console.log(ansis.dim(`Published: ${formatDueDate(advisory.publishedAt)}`));
}
console.log();
for (const fn of advisory.vulnerableFunctions) {
console.log(` • ${fn}`);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Sanitize external advisory fields and handle empty function lists gracefully to avoid empty headers and stray newlines.

Suggested change
export function printAdvisoryBlock(advisory: AdvisoryInformation): void {
console.log();
console.log(ansis.bold(`Vulnerable Functions (${advisory.advisoryId})`));
if (advisory.publishedAt) {
console.log(ansis.dim(`Published: ${formatDueDate(advisory.publishedAt)}`));
}
console.log();
for (const fn of advisory.vulnerableFunctions) {
console.log(` • ${fn}`);
}
}
export function printAdvisoryBlock(advisory: AdvisoryInformation): void {
console.log();
console.log(ansis.bold(`Vulnerable Functions (${sanitizeText(advisory.advisoryId)})`));
if (advisory.publishedAt) {
console.log(ansis.dim(`Published: ${formatDueDate(advisory.publishedAt)}`));
}
if (advisory.vulnerableFunctions.length > 0) {
console.log();
for (const fn of advisory.vulnerableFunctions) {
console.log(` • ${sanitizeText(fn)}`);
}
}
}

Comment thread src/utils/formatting.ts
Comment on lines +266 to +270
// Vulnerable functions (SCA issues with an OSV-linked advisory), compact form
if (issue.advisoryInformation) {
console.log();
console.log(ansis.dim(`Vulnerable functions: ${summarizeFunctions(issue.advisoryInformation.vulnerableFunctions)}`));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚪ LOW RISK

Suggestion: Only render the vulnerable functions line if there are functions to display.

Suggested change
// Vulnerable functions (SCA issues with an OSV-linked advisory), compact form
if (issue.advisoryInformation) {
console.log();
console.log(ansis.dim(`Vulnerable functions: ${summarizeFunctions(issue.advisoryInformation.vulnerableFunctions)}`));
}
// Vulnerable functions (SCA issues with an OSV-linked advisory), compact form
if (issue.advisoryInformation?.vulnerableFunctions?.length) {
console.log();
console.log(ansis.dim(`Vulnerable functions: ${summarizeFunctions(issue.advisoryInformation.vulnerableFunctions)}`));
}

zhamborova
zhamborova previously approved these changes Jul 28, 2026
External OSV advisory strings (function names, advisory ID) were printed
raw, letting a crafted advisory smuggle terminal control sequences
(CWE-150); route them through sanitizeText() like other repo-derived
output. Also gate the vulnerable-functions line on a non-empty list and
split printFindingCard into two helpers to stay under the line-count
limit, and add the missing test for advisory-block suppression when a
finding has a linked issue.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for displaying vulnerable and affected functions (SCA) in the issue, issues, pull-request, finding, and findings commands. It includes both compact card summaries and detailed advisory blocks, with JSON output support and a pinned API update to access these new fields.

While the implementation aligns with most acceptance criteria, including the logic to suppress duplicate advisory blocks when Codacy issue data is already present, there are security gaps regarding the sanitization of terminal output. Several fields in the findings list view (likelihood, effortToFix, cve, and cwe) are currently bypassing the sanitizeText() utility. Addressing these is necessary to ensure full protection against terminal control character injection (CWE-150).

About this PR

  • The test suite for advisoryInformation is highly repetitive across findings.test.ts, finding.test.ts, issue.test.ts, and pull-request.test.ts. Consider creating a shared test helper to verify card output and JSON structures for advisories to improve maintainability and reduce future duplication.

Test suggestions

  • Verify compact vulnerable functions line appears on issue cards when advisory data is present
  • Verify compact vulnerable functions line appears in findings list view when advisory data is present
  • Verify vulnerable functions list is truncated to 3 items with correct '+N more' suffix in compact views
  • Verify full advisory block appears in 'issue' and 'pull-request --issue' detail views
  • Verify advisory block appears in 'finding' detail view for non-Codacy (SCA/dependency) findings
  • Verify advisory block is suppressed in 'finding' detail view if a linked Codacy issue is present (avoiding duplicates)
  • Verify advisoryInformation is correctly projected in --output json for all commands
  • Verify terminal control characters in function names or advisory IDs are neutralized via sanitization

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread src/commands/findings.ts
Comment on lines +111 to +112
if (item.cve) parts.push(ansis.dim(item.cve));
else if (item.cwe) parts.push(ansis.dim(`CWE-${item.cwe}`));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Neutralize terminal control characters in item.cve and item.cwe using sanitizeText().

Suggested change
if (item.cve) parts.push(ansis.dim(item.cve));
else if (item.cwe) parts.push(ansis.dim(`CWE-${item.cwe}`));
if (item.cve) parts.push(ansis.dim(sanitizeText(item.cve)));
else if (item.cwe) parts.push(ansis.dim(`CWE-${sanitizeText(item.cwe)}`));

Comment thread src/commands/findings.ts
Comment on lines 93 to +96
const penTestParts = [item.likelihood, item.effortToFix].filter(
(v) => v && v !== "not_applicable",
) as string[];
if (penTestParts.length > 0) line1Parts.push(penTestParts.join(" "));
if (penTestParts.length > 0) parts.push(penTestParts.join(" "));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Neutralize terminal control characters in likelihood and effortToFix using sanitizeText() before they are pushed to the header parts. You can update buildFindingHeaderLine in src/commands/findings.ts to map these values through the sanitizer before joining them.

Comment thread src/commands/findings.ts
function printFindingCard(item: SrmItem, showRepo: boolean): void {
const separator = ansis.dim("─".repeat(40));
// Line 1: Priority | SecurityCategory ScanType | Likelihood EffortToFix | Repository
function buildFindingHeaderLine(item: SrmItem, showRepo: boolean): string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 MEDIUM RISK

Suggestion: The logic for constructing finding headers and status lines is shared between the finding (detail) and findings (list) commands. To avoid rendering inconsistencies, move buildFindingHeaderLine and buildFindingStatusLine to src/utils/formatting.ts and use them in both command files.

See Clone in Codacy

@alerizzo
alerizzo merged commit c26ff79 into main Jul 29, 2026
4 checks passed
@alerizzo
alerizzo deleted the feature/source-id-issues-OD-296 branch July 29, 2026 10:48
@github-actions github-actions Bot mentioned this pull request Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants