-
Notifications
You must be signed in to change notification settings - Fork 42
(old) feat: output security reports as JSON when requested #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
50b72c0
3faf18f
3493140
1e3a81f
009798b
9e2a85c
eb9af7c
04444c1
231ba24
8a94119
62228e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import { promises as fs } from 'fs'; | |
| import path from 'path'; | ||
| import { getAuditScope } from './filesystem.js'; | ||
| import { findLineNumbers } from './security.js'; | ||
| import { parseMarkdownToDict } from './parser.js'; | ||
|
|
||
| import { runPoc } from './poc.js'; | ||
|
|
||
|
|
@@ -64,6 +65,36 @@ server.tool( | |
| (input: { filePath: string }) => runPoc(input) | ||
| ); | ||
|
|
||
| server.tool( | ||
| 'convert_report_to_json', | ||
| 'Converts the Markdown security report into a JSON file named security_report.json in the .gemini_security folder.', | ||
| {} as any, | ||
| (async () => { | ||
| try { | ||
| const reportPath = path.join(process.cwd(), '.gemini_security/DRAFT_SECURITY_REPORT.md'); | ||
| const outputPath = path.join(process.cwd(), '.gemini_security/security_report.json'); | ||
|
|
||
| const content = await fs.readFile(reportPath, 'utf-8'); | ||
| const results = parseMarkdownToDict(content); | ||
|
|
||
| await fs.writeFile(outputPath, JSON.stringify(results, null, 2)); | ||
|
|
||
| return { | ||
| content: [{ | ||
| type: 'text', | ||
| text: `Successfully created JSON report at ${outputPath}` | ||
| }] | ||
| }; | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| return { | ||
| content: [{ type: 'text', text: `Error converting to JSON: ${message}` }], | ||
| isError: true | ||
| }; | ||
| } | ||
| }) as any | ||
| ); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🟡 Casting the function to `any` using `as any` bypasses TypeScript's type safety checks. It would be more robust to define and use a specific type for the server tool implementation to ensure type safety and prevent potential runtime errors.
|
||
| server.registerPrompt( | ||
| 'security:note-adder', | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2025 Google LLC | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, it, expect } from 'vitest'; | ||
| import { parseMarkdownToDict } from './parser.js'; | ||
|
|
||
| describe('parseMarkdownToDict', () => { | ||
| it('should parse a standard security vulnerability correctly', () => { | ||
| const mdContent = ` | ||
| Vulnerability: Hardcoded API Key | ||
| Vulnerability Type: Security | ||
| Severity: Critical | ||
| Source Location: config/settings.js:15-15 | ||
| Line Content: const KEY = "sk_live_12345"; | ||
| Description: A production secret was found hardcoded in the source. | ||
| Recommendation: Move the secret to an environment variable. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results).toHaveLength(1); | ||
| expect(results[0]).toMatchObject({ | ||
| vulnerability: 'Hardcoded API Key', | ||
| vulnerabilityType: 'Security', | ||
| severity: 'Critical', | ||
| lineContent: 'const KEY = "sk_live_12345";', | ||
| sourceLocation: { | ||
| file: 'config/settings.js', | ||
| startLine: 15, | ||
| endLine: 15 | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| it('should parse a privacy violation with Sink and Data Type', () => { | ||
| const mdContent = ` | ||
| Vulnerability: PII Leak in Logs | ||
| Vulnerability Type: Privacy | ||
| Severity: Medium | ||
| Source Location: src/auth.ts:22 | ||
| Sink Location: console.log:45 | ||
| Data Type: Email Address | ||
| Line Content: logger.info("User logged in: " + user.email); | ||
| Description: Unmasked email addresses are being written to application logs. | ||
| Recommendation: Redact the email address before logging. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results).toHaveLength(1); | ||
| expect(results[0]).toMatchObject({ | ||
| sinkLocation: { | ||
| file: 'console.log', | ||
| startLine: 45, | ||
| endLine: 45 | ||
| }, | ||
| dataType: 'Email Address' | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle multiple vulnerabilities in one file', () => { | ||
| const mdContent = ` | ||
| Vulnerability: SQL Injection | ||
| Vulnerability Type: Security | ||
| Severity: High | ||
| Source Location: db.js:10 | ||
| Line Content: query = "SELECT * FROM users WHERE id = " + id; | ||
| Description: Raw input used in query. | ||
| Recommendation: Use parameterized queries. | ||
|
|
||
| Vulnerability: Reflected XSS | ||
| Vulnerability Type: Security | ||
| Severity: Medium | ||
| Source Location: app.js:100 | ||
| Line Content: res.send("Hello " + req.query.name); | ||
| Description: User input rendered without escaping. | ||
| Recommendation: Use a templating engine with auto-escaping. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
| expect(results).toHaveLength(2); | ||
| expect(results[0].vulnerability).toBe('SQL Injection'); | ||
| expect(results[1].vulnerability).toBe('Reflected XSS'); | ||
| }); | ||
|
|
||
| it('should handle markdown formatting like bolding and bullets', () => { | ||
| const mdContent = ` | ||
| * **Vulnerability:** Hardcoded Secret | ||
| - **Severity:** High | ||
| * **Source Location:** \`index.js:5-10\` | ||
| - **Line Content:** \`\`\`javascript | ||
| const secret = "password"; | ||
| \`\`\` | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results[0].vulnerability).toBe('Hardcoded Secret'); | ||
| expect(results[0].severity).toBe('High'); | ||
| expect(results[0].sourceLocation.file).toBe('index.js'); | ||
| expect(results[0].lineContent).toBe('const secret = "password";'); | ||
| }); | ||
|
|
||
| it('should return empty array if no "Vulnerability:" trigger is found', () => { | ||
| const mdContent = "This is a summary report with no specific findings."; | ||
| const results = parseMarkdownToDict(mdContent); | ||
| expect(results).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('should handle missing line numbers and sink location', () => { | ||
| const mdContent = ` | ||
| Vulnerability: Missing Line Numbers | ||
| Vulnerability Type: Security | ||
| Severity: High | ||
| Source Location: src/index.ts | ||
| Line Content: const apiKey = process.env.API_KEY; | ||
| Description: Source location without line numbers. | ||
| Recommendation: Verify the vulnerability details. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results).toHaveLength(1); | ||
| expect(results[0]).toMatchObject({ | ||
| vulnerability: 'Missing Line Numbers', | ||
| vulnerabilityType: 'Security', | ||
| severity: 'High', | ||
| lineContent: 'const apiKey = process.env.API_KEY;' | ||
| }); | ||
| expect(results[0].sourceLocation.file).toBe('src/index.ts'); | ||
| }); | ||
|
|
||
| it('should handle missing end line number', () => { | ||
| const mdContent = ` | ||
| Vulnerability: No End Line | ||
| Vulnerability Type: Security | ||
| Severity: Medium | ||
| Source Location: app.js:42 | ||
| Line Content: res.send(userInput); | ||
| Description: Source location with only start line number. | ||
| Recommendation: Check this line. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results).toHaveLength(1); | ||
| expect(results[0].sourceLocation).toMatchObject({ | ||
| file: 'app.js', | ||
| startLine: 42 | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle missing sink location', () => { | ||
| const mdContent = ` | ||
| Vulnerability: No Sink Info | ||
| Vulnerability Type: Privacy | ||
| Severity: Low | ||
| Source Location: logger.ts:15 | ||
| Data Type: User ID | ||
| Line Content: console.log(user.id); | ||
| Description: Vulnerability without sink location details. | ||
| Recommendation: Use proper logging. | ||
| `; | ||
|
|
||
| const results = parseMarkdownToDict(mdContent); | ||
|
|
||
| expect(results).toHaveLength(1); | ||
| expect(results[0]).toMatchObject({ | ||
| vulnerability: 'No Sink Info', | ||
| vulnerabilityType: 'Privacy', | ||
| severity: 'Low' | ||
| }); | ||
| expect(results[0].dataType).toBe('User ID'); | ||
| expect( | ||
| results[0].sinkLocation === undefined || | ||
| (results[0].sinkLocation?.file === null && | ||
| results[0].sinkLocation?.startLine === null && | ||
| results[0].sinkLocation?.endLine === null) | ||
| ).toBe(true); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,113 @@ | ||||||||||
| /** | ||||||||||
| * @license | ||||||||||
| * Copyright 2025 Google LLC | ||||||||||
| * SPDX-License-Identifier: Apache-2.0 | ||||||||||
| */ | ||||||||||
|
|
||||||||||
| export interface Location { | ||||||||||
| file: string | null; | ||||||||||
| startLine: number | null; | ||||||||||
| endLine: number | null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| export interface Finding { | ||||||||||
| vulnerability: string | null; | ||||||||||
| vulnerabilityType: string | null; | ||||||||||
| severity: string | null; | ||||||||||
| dataType: string | null; | ||||||||||
| sourceLocation: Location; | ||||||||||
| sinkLocation: Location; | ||||||||||
| lineContent: string | null; | ||||||||||
| description: string | null; | ||||||||||
| recommendation: string | null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Parses a markdown string containing security findings into a structured format. | ||||||||||
| * The markdown should follow a specific format where each finding starts with "Vulnerability:" and includes fields like "Severity:", "Source Location:", etc. | ||||||||||
| * The function uses regular expressions to extract the relevant information and returns an array of findings. | ||||||||||
| * | ||||||||||
| * @param content - The markdown string to parse. | ||||||||||
| * @returns An array of structured findings extracted from the markdown. | ||||||||||
| */ | ||||||||||
| function parseLocation(locationStr: string | null): Location { | ||||||||||
| if (!locationStr) { | ||||||||||
| return { file: null, startLine: null, endLine: null }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const cleanStr = locationStr.replace(/`/g, '').trim(); | ||||||||||
| // Regex: path/file.ext:start-end or path/file.ext:line | ||||||||||
| // Matches: file.ext:12-34 OR file.ext:12 OR file.ext | ||||||||||
| const match = cleanStr.match(/^([^:]+)(?::(\d+)(?:-(\d+))?)?$/); | ||||||||||
|
|
||||||||||
| if (match) { | ||||||||||
| const filePath = match[1].trim(); | ||||||||||
| let start: number | null = null; | ||||||||||
| let end: number | null = null; | ||||||||||
| if (match[2] && match[3]) { | ||||||||||
| start = parseInt(match[2], 10); | ||||||||||
| end = parseInt(match[3], 10); | ||||||||||
| } else if (match[2]) { | ||||||||||
| start = parseInt(match[2], 10); | ||||||||||
| end = start; | ||||||||||
| } | ||||||||||
| return { file: filePath, startLine: start, endLine: end }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return { file: cleanStr, startLine: null, endLine: null }; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Parses a markdown string containing security findings into a structured format. | ||||||||||
| * The markdown should follow a specific format where each finding starts with "Vulnerability:" and includes fields like "Severity:", "Source Location:", etc. | ||||||||||
| * The function uses regular expressions to extract the relevant information and returns an array of findings. | ||||||||||
| * | ||||||||||
| * @param content - The markdown string to parse. | ||||||||||
| * @returns An array of structured findings extracted from the markdown. | ||||||||||
| */ | ||||||||||
| export function parseMarkdownToDict(content: string): Finding[] { | ||||||||||
| const findings: Finding[] = []; | ||||||||||
|
|
||||||||||
| // Remove markdown bullet points (only at line start), markdown emphasis, and preserve hyphens/underscores in text | ||||||||||
| const cleanContent = content | ||||||||||
| .replace(/^\s*[\*\-]\s*/gm, '') // Remove bullet points at line start | ||||||||||
| .replace(/\*\*/g, ''); // Remove ** markdown | ||||||||||
|
|
||||||||||
| // Split by "Vulnerability:" preceded by newline | ||||||||||
| const sections = cleanContent.split(/\n(?=#{1,6} |\s*Vulnerability:)/); | ||||||||||
|
|
||||||||||
| for (let section of sections) { | ||||||||||
| section = section.trim(); | ||||||||||
| if (!section || !section.includes("Vulnerability:")) continue; | ||||||||||
|
|
||||||||||
| const extract = (label: string): string | null => { | ||||||||||
| const fieldNames = 'Vulnerability|Severity|Source|Sink|Data|Line|Description|Recommendation'; | ||||||||||
jajanet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| const patternStr = `(?:-?\\s*\\**)?${label}\\**:\\s*([\\s\\S]*?)(?=\\n(?:-?\\s*\\**)?(?:${fieldNames})|$)`; | ||||||||||
| const pattern = new RegExp(patternStr, 'i'); | ||||||||||
|
||||||||||
| const pattern = new RegExp(patternStr, 'i'); | |
| const fieldNames = 'Vulnerability Type|Severity|Source Location|Sink Location|Data Type|Line Content|Description|Recommendation'; | |
| const patternStr = `(?:-?\\\\s*\\\\**)?${label}\\\\**:\\\\s*([\\\\s\\\\S]*?)(?=\\\\n(?:-?\\\\s*\\\\**)?(?:${fieldNames})|$)`; | |
| const pattern = new RegExp(patternStr, 'i'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we fix this?
jajanet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
jajanet marked this conversation as resolved.
Show resolved
Hide resolved
jajanet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if there is an error in any of these extraction methods? It might be worth considering failing gracefully here as the rest of the extraction might still produce a reasonable report?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns null! Examples:
[
{
"vulnerability": "Cross-Site Scripting (XSS)",
"vulnerabilityType": "Security",
"severity": "High",
"dataType": null, // <-- only relevant to privacy issues
"sourceLocation": {
"file": "js/src/util.js",
"startLine": 120,
"endLine": 120
},
"sinkLocation": {
"file": null,
"startLine": null,
"endLine": null
},
"lineContent": "`const $selector = $(selector)`",
"description": "The `getSelectorFromElement` function is vulnerable to DOM-based XSS. It constructs a jQuery selector from the `data-target` or `href` attributes of an element without proper sanitization. An attacker can inject malicious JavaScript code into these attributes, which will then be executed when the selector is processed by jQuery.",
"recommendation": "Sanitize the selector before passing it to jQuery. For example, ensure that the selector only contains valid CSS selector characters. A simple fix is to escape the selector using a library like `CSS.escape()` if available, or to validate the selector against a regex for valid characters. Given the context of this file, a more robust solution would be to ensure that any user-provided input is properly encoded before being used in a selector."
}
]and
[
{
"vulnerability": "Cross-Site Scripting (XSS)",
"vulnerabilityType": "Security",
"severity": "High",
"dataType": null,
"sourceLocation": {
"file": "bin/mk-server",
"startLine": null, // sometimes missing
"endLine": null
},
"sinkLocation": {
"file": null,
"startLine": null,
"endLine": null
},
"lineContent": "`res.end \"<h1>User No: <span style='color: red;'>\" + req.params.id + \"</span></h1>\"`",
"description": "The application is vulnerable to reflected XSS. The `id` parameter from the URL is directly embedded into the HTML response without any sanitization. This allows an attacker to inject malicious scripts into the response, which will be executed in the context of the user's browser.",
"recommendation": "Sanitize the `req.params.id` variable before embedding it in the HTML response. Use a library like `he` to encode HTML entities."
}
]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not sure what you mean by "can we fix this?" and can't reply directly to that comment -- do you mean pull it out of the function? I did add caching the regex pattern (for performance) and small readability changes like pulling it out of the function and using a list for field names, but that might not be what you're referring to? :
const FIELD_NAMES = [
'Vulnerability',
'Severity',
'Source',
'Sink',
'Data',
'Line',
'Description',
'Recommendation',
].join('|'); // add more labels here
const patternCache = new Map<string, RegExp>();
const escapeRegExp = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
...
/**
* Parses a markdown string containing security findings into a structured format.
* The markdown should follow a specific format where each finding starts with "Vulnerability:" and includes fields like "Severity:", "Source Location:", etc.
* The function uses regular expressions to extract the relevant information and returns an array of findings.
*
* @param content - The markdown string to parse.
* @returns An array of structured findings extracted from the markdown.
*/
function extractFromSection(section: string, label: string): string | null {
const pattern = buildPattern(label);
const match = section.match(pattern);
return match ? match[1].trim() : null;
};
...
const rawSource = extractFromSection(section, "Source Location");
...
Uh oh!
There was an error while loading. Please reload this page.