Skip to content

Commit 720d2c8

Browse files
authored
Merge pull request #192 from advanced-security/copilot/fix-sarif-cwe-tag-bug
Fix CWE tag mapping for IDs with leading zeros
2 parents 130ae34 + 2d3ac19 commit 720d2c8

File tree

6 files changed

+117
-7
lines changed

6 files changed

+117
-7
lines changed

__tests__/main.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,79 @@
1+
import {normalizeCweId} from '../src/utils'
2+
13
describe('main', () => {
24
it('placeholder test', () => {
35
// This is a placeholder test to prevent Jest from failing
46
// TODO: Add proper unit tests for the main module
57
expect(true).toBe(true)
68
})
9+
10+
describe('CWE ID normalization', () => {
11+
it('should handle CWE IDs with leading zeros', () => {
12+
// Test that 099 maps to 99
13+
const normalizedId = normalizeCweId('099')
14+
expect(normalizedId).toBe('99')
15+
})
16+
17+
it('should handle CWE IDs without leading zeros', () => {
18+
// Test that 89 maps to 89
19+
const normalizedId = normalizeCweId('89')
20+
expect(normalizedId).toBe('89')
21+
})
22+
23+
it('should handle CWE IDs with multiple leading zeros', () => {
24+
// Test that 020 maps to 20
25+
const normalizedId = normalizeCweId('020')
26+
expect(normalizedId).toBe('20')
27+
})
28+
29+
it('should return null for non-numeric CWE IDs', () => {
30+
// Test that invalid CWE IDs return null
31+
const normalizedId = normalizeCweId('abc')
32+
expect(normalizedId).toBeNull()
33+
})
34+
35+
it('should return null for empty strings', () => {
36+
// Test that empty strings return null
37+
const normalizedId = normalizeCweId('')
38+
expect(normalizedId).toBeNull()
39+
})
40+
41+
it('should return null for strings with only spaces', () => {
42+
// Test that strings with only spaces return null
43+
const normalizedId = normalizeCweId(' ')
44+
expect(normalizedId).toBeNull()
45+
})
46+
47+
it('should handle strings with leading/trailing spaces', () => {
48+
// Test that strings with spaces are parsed correctly
49+
const normalizedId = normalizeCweId(' 99 ')
50+
expect(normalizedId).toBe('99')
51+
})
52+
53+
it('should return null for negative numbers', () => {
54+
// Test that negative numbers return null (CWE IDs should be positive)
55+
const normalizedId = normalizeCweId('-99')
56+
expect(normalizedId).toBeNull()
57+
})
58+
59+
it('should handle strings with mixed alphanumeric characters (parseInt is lenient)', () => {
60+
// Test that mixed alphanumeric strings are parsed leniently
61+
// parseInt stops at first non-numeric character, so '99abc' becomes 99
62+
// This is acceptable for our use case as malformed tags would be rare
63+
const normalizedId = normalizeCweId('99abc')
64+
expect(normalizedId).toBe('99')
65+
})
66+
67+
it('should handle zero', () => {
68+
// Test that zero is handled correctly
69+
const normalizedId = normalizeCweId('0')
70+
expect(normalizedId).toBe('0')
71+
})
72+
73+
it('should handle zero with leading zeros', () => {
74+
// Test that zero with leading zeros is handled correctly
75+
const normalizedId = normalizeCweId('000')
76+
expect(normalizedId).toBe('0')
77+
})
78+
})
779
})

dist/index.js

Lines changed: 21 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/main.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as core from '@actions/core'
55
import {DOMParser} from '@xmldom/xmldom'
66
import * as xpath from 'xpath'
77
import {JSONPath} from 'jsonpath-plus'
8-
import {LogLevel, log} from './utils'
8+
import {LogLevel, log, normalizeCweId} from './utils'
99

1010
let sarifFilePath: string
1111
let outputFilePath: string
@@ -103,9 +103,15 @@ JSONPath({
103103
for (const tag of tags) {
104104
if (tag.startsWith(codeQlCweTagPrefix)) {
105105
const cweId = tag.replace(codeQlCweTagPrefix, '')
106-
if (cweIds.includes(cweId)) {
106+
// Normalize CWE ID by converting to integer to remove leading zeros
107+
const normalizedCweId = normalizeCweId(cweId)
108+
// Skip if the CWE ID is not a valid number
109+
if (normalizedCweId === null) {
110+
continue
111+
}
112+
if (cweIds.includes(normalizedCweId)) {
107113
tags.push(securityStandardTag)
108-
tags.push(...cweCategories[cweId])
114+
tags.push(...cweCategories[normalizedCweId])
109115
return
110116
}
111117
}

src/utils.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,16 @@ export function log(message: string, level = LogLevel.Info): void {
4141
}
4242
}
4343
}
44+
45+
/**
46+
* Normalize a CWE ID by removing leading zeros
47+
* @param cweId - The CWE ID string (e.g., "099", "020", "89")
48+
* @returns The normalized CWE ID string (e.g., "99", "20", "89") or null if invalid
49+
*/
50+
export function normalizeCweId(cweId: string): string | null {
51+
const parsedCweId = parseInt(cweId, 10)
52+
if (Number.isNaN(parsedCweId) || parsedCweId < 0) {
53+
return null
54+
}
55+
return String(parsedCweId)
56+
}

test-data/webgoat-with-security-standard-tag.sarif.expected

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)