Skip to content

Commit 74cbbec

Browse files
committed
fix: Sanitize advisory data in vulnerable functions output OD-296
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.
1 parent 5838a71 commit 74cbbec

4 files changed

Lines changed: 70 additions & 31 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@codacy/codacy-cloud-cli": patch
3+
---
4+
5+
Sanitize vulnerable/affected function names and the advisory ID (`CommitIssue.advisoryInformation` / `SrmItem.advisoryInformation`) before printing them in `issue`, `issues`, `pull-request --issue`, `finding`, and `findings`. These values come from the linked OSV advisory, so — like other repository-derived output — they are now passed through `sanitizeText()` to strip ANSI/OSC control bytes (CWE-150) instead of being printed raw.

src/commands/finding.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,30 @@ describe("finding command", () => {
651651
expect.stringContaining('"lodash.merge"'),
652652
);
653653
});
654+
655+
it("should suppress its own advisory block for a Codacy-source finding with a linked issue, deferring to the issue's block", async () => {
656+
vi.mocked(SecurityService.getSecurityItem).mockResolvedValue({
657+
data: { ...mockCodacyFinding, advisoryInformation: mockAdvisoryInformation },
658+
} as any);
659+
vi.mocked(AnalysisService.getIssue).mockResolvedValue({
660+
data: { ...mockQualityIssue, advisoryInformation: mockAdvisoryInformation },
661+
} as any);
662+
vi.mocked(ToolsService.getPattern).mockResolvedValue({
663+
data: mockPattern,
664+
} as any);
665+
vi.mocked(FileService.getFileContent).mockResolvedValue({
666+
data: mockFileLines,
667+
} as any);
668+
669+
const program = createProgram();
670+
await program.parseAsync([
671+
"node", "test", "finding", "gh", "test-org", "def-456-codacy",
672+
]);
673+
674+
const output = getAllOutput();
675+
// Rendered once via printIssueCodeContext (the linked issue's block), not duplicated by finding.ts's own printAdvisoryBlock call
676+
expect(output.match(/Vulnerable Functions \(CVE-2021-23337\)/g)).toHaveLength(1);
677+
});
654678
});
655679

656680
describe("--ignore option", () => {

src/commands/findings.ts

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -77,61 +77,69 @@ function normalizeScanType(input: string): string {
7777
);
7878
}
7979

80-
function printFindingCard(item: SrmItem, showRepo: boolean): void {
81-
const separator = ansis.dim("─".repeat(40));
80+
// Line 1: Priority | SecurityCategory ScanType | Likelihood EffortToFix | Repository
81+
function buildFindingHeaderLine(item: SrmItem, showRepo: boolean): string {
8282
const pipe = ` ${ansis.dim("|")} `;
83-
84-
console.log();
85-
86-
// Line 1: Priority | SecurityCategory ScanType | Likelihood EffortToFix | Repository
87-
const line1Parts: string[] = [colorPriority(item.priority)];
83+
const parts: string[] = [colorPriority(item.priority)];
8884

8985
const catParts = [
9086
sanitizeText(item.securityCategory),
9187
item.scanType ? ansis.dim(sanitizeText(item.scanType)) : undefined,
9288
]
9389
.filter(Boolean)
9490
.join(" ");
95-
if (catParts) line1Parts.push(catParts);
91+
if (catParts) parts.push(catParts);
9692

9793
const penTestParts = [item.likelihood, item.effortToFix].filter(
9894
(v) => v && v !== "not_applicable",
9995
) as string[];
100-
if (penTestParts.length > 0) line1Parts.push(penTestParts.join(" "));
96+
if (penTestParts.length > 0) parts.push(penTestParts.join(" "));
10197

102-
if (showRepo && item.repository) line1Parts.push(ansis.dim(sanitizeText(item.repository)));
98+
if (showRepo && item.repository) parts.push(ansis.dim(sanitizeText(item.repository)));
10399

104100
const idLabel = ansis.hex("#555555")(item.id);
105-
console.log(line1Parts.join(pipe) + ` ${idLabel}`);
106-
107-
// Line 2: Title
108-
console.log(sanitizeText(item.title));
109-
if (item.affectedTargets) console.log(ansis.dim(sanitizeText(item.affectedTargets)));
110-
console.log();
101+
return parts.join(pipe) + ` ${idLabel}`;
102+
}
111103

112-
// Line 3: Status DueAt | CVE/CWE | AffectedVersion → FixedVersion | Application | AffectedTargets
113-
const line3Parts: string[] = [
104+
// Line 3: Status DueAt | CVE/CWE | AffectedVersion → FixedVersion | Application
105+
function buildFindingStatusLine(item: SrmItem, hasChains: boolean): string {
106+
const pipe = ` ${ansis.dim("|")} `;
107+
const parts: string[] = [
114108
`${colorStatus(item.status)} ${ansis.dim(formatDueDate(item.dueAt))}`,
115109
];
116110

117-
if (item.cve) line3Parts.push(ansis.dim(item.cve));
118-
else if (item.cwe) line3Parts.push(ansis.dim(`CWE-${item.cwe}`));
111+
if (item.cve) parts.push(ansis.dim(item.cve));
112+
else if (item.cwe) parts.push(ansis.dim(`CWE-${item.cwe}`));
119113

120114
// When dependency chains are present they carry the vulnerable package and
121115
// fixed version on their own line, so the redundant version segment is dropped.
122-
const hasChains = !!item.dependencyChains?.length;
123116
if (!hasChains) {
124117
const versionSegment = formatVersionSegment(
125118
item.affectedVersion,
126119
item.fixedVersion,
127120
{ includeUpdatePrefix: true },
128121
);
129-
if (versionSegment) line3Parts.push(ansis.dim(versionSegment));
122+
if (versionSegment) parts.push(ansis.dim(versionSegment));
130123
}
131124

132-
if (item.application) line3Parts.push(ansis.dim(sanitizeText(item.application)));
125+
if (item.application) parts.push(ansis.dim(sanitizeText(item.application)));
133126

134-
console.log(line3Parts.join(pipe));
127+
return parts.join(pipe);
128+
}
129+
130+
function printFindingCard(item: SrmItem, showRepo: boolean): void {
131+
const separator = ansis.dim("─".repeat(40));
132+
133+
console.log();
134+
console.log(buildFindingHeaderLine(item, showRepo));
135+
136+
// Line 2: Title
137+
console.log(sanitizeText(item.title));
138+
if (item.affectedTargets) console.log(ansis.dim(sanitizeText(item.affectedTargets)));
139+
console.log();
140+
141+
const hasChains = !!item.dependencyChains?.length;
142+
console.log(buildFindingStatusLine(item, hasChains));
135143

136144
// Line 4: dependency import chain (SCA findings with dependencyChains)
137145
if (hasChains) {
@@ -143,7 +151,7 @@ function printFindingCard(item: SrmItem, showRepo: boolean): void {
143151
}
144152

145153
// Vulnerable functions (findings with an OSV-linked advisory), compact form
146-
if (item.advisoryInformation) {
154+
if (item.advisoryInformation?.vulnerableFunctions?.length) {
147155
console.log(
148156
ansis.dim(`Vulnerable functions: ${summarizeFunctions(item.advisoryInformation.vulnerableFunctions)}`),
149157
);

src/utils/formatting.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ export function printIssueCard(
264264
}
265265

266266
// Vulnerable functions (SCA issues with an OSV-linked advisory), compact form
267-
if (issue.advisoryInformation) {
267+
if (issue.advisoryInformation?.vulnerableFunctions?.length) {
268268
console.log();
269269
console.log(ansis.dim(`Vulnerable functions: ${summarizeFunctions(issue.advisoryInformation.vulnerableFunctions)}`));
270270
}
@@ -278,7 +278,7 @@ export function printIssueCard(
278278
* capping at 3 entries with a "+N more" suffix for longer lists.
279279
*/
280280
export function summarizeFunctions(fns: string[], limit = 3): string {
281-
const shown = fns.slice(0, limit).join(", ");
281+
const shown = fns.slice(0, limit).map(sanitizeText).join(", ");
282282
const more = fns.length > limit ? ` (+${fns.length - limit} more)` : "";
283283
return `${shown}${more}`;
284284
}
@@ -634,13 +634,15 @@ export function printCveBlock(cve: CveRecord): void {
634634
*/
635635
export function printAdvisoryBlock(advisory: AdvisoryInformation): void {
636636
console.log();
637-
console.log(ansis.bold(`Vulnerable Functions (${advisory.advisoryId})`));
637+
console.log(ansis.bold(`Vulnerable Functions (${sanitizeText(advisory.advisoryId)})`));
638638
if (advisory.publishedAt) {
639639
console.log(ansis.dim(`Published: ${formatDueDate(advisory.publishedAt)}`));
640640
}
641-
console.log();
642-
for (const fn of advisory.vulnerableFunctions) {
643-
console.log(` • ${fn}`);
641+
if (advisory.vulnerableFunctions.length > 0) {
642+
console.log();
643+
for (const fn of advisory.vulnerableFunctions) {
644+
console.log(` • ${sanitizeText(fn)}`);
645+
}
644646
}
645647
}
646648

0 commit comments

Comments
 (0)